-
Notifications
You must be signed in to change notification settings - Fork 12
Header cell with unit tests, styling, and new interfaces #8
Changes from 4 commits
23008b2
57ab9ad
c657fd2
0a2542b
a28e905
4965fd0
d4e1d5c
e20b1e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
import WidgetBase from '@dojo/widget-core/WidgetBase'; | ||
import { RegistryMixin } from '@dojo/widget-core/mixins/Registry'; | ||
import { v } from '@dojo/widget-core/d'; | ||
import { HasColumn, HasSortDetail, HasSortEvent } from './interfaces'; | ||
import { ThemeableMixin, theme, ThemeableProperties } from '@dojo/widget-core/mixins/Themeable'; | ||
|
||
import * as headerCellClasses from './styles/headerCell.css'; | ||
|
||
export interface HeaderCellProperties extends ThemeableProperties, HasColumn, HasSortDetail, HasSortEvent { } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
|
||
export const HeaderCellBase = ThemeableMixin(RegistryMixin(WidgetBase)); | ||
|
||
@theme(headerCellClasses) | ||
class HeaderCell extends HeaderCellBase<HeaderCellProperties> { | ||
onSortRequest(): void { | ||
const { | ||
key = '', | ||
column, | ||
sortDetail, | ||
onSortRequest | ||
} = this.properties; | ||
|
||
if (onSortRequest && (column.sortable || !column.hasOwnProperty('sortable'))) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you think it would be better to not attach the handler if the column is not sortable? |
||
onSortRequest({ | ||
columnId: key, | ||
descending: Boolean(sortDetail && sortDetail.columnId === key && !sortDetail.descending) | ||
}); | ||
} | ||
} | ||
|
||
render() { | ||
const { | ||
key, | ||
column, | ||
sortDetail | ||
} = this.properties; | ||
|
||
const classes = [ headerCellClasses.headerCell, column.sortable !== false ? headerCellClasses.sortable : null ]; | ||
|
||
const sortClasses = [ | ||
sortDetail ? headerCellClasses.sortArrow : null, | ||
sortDetail && sortDetail.descending ? headerCellClasses.sortArrowDown : null, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think clarity is retained if this line and the following are collapsed into There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since these all depend on sortDetail, I did a ternary that either creates this array or an empty array, then used your ternary approach. It does read much better. Thanks. |
||
sortDetail && !sortDetail.descending ? headerCellClasses.sortArrowUp : null | ||
]; | ||
|
||
return v('th', { | ||
role: 'columnheader', | ||
onclick: this.onSortRequest, | ||
classes: this.classes(...classes) | ||
}, [ | ||
v('span', [ column.label || column.id ]), | ||
sortDetail && sortDetail.columnId === key ? v('div', { | ||
role: 'presentation', | ||
classes: this.classes(...sortClasses) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here re building up the classes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks |
||
}) : null | ||
]); | ||
} | ||
} | ||
|
||
export default HeaderCell; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
@import 'headerCell.css'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you're going to offer up a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure how to exclude this from the poscss task. Right now I'm using the pattern from dojo/widgets that just overwrites the file with its original version. |
||
@import 'cell.css'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
.headerCell { | ||
composes: cell from './shared/cell.css'; | ||
position: relative; | ||
} | ||
|
||
.sortable { | ||
cursor: pointer; | ||
} | ||
|
||
.sortArrow { | ||
position: absolute; | ||
top: 50%; | ||
transform: translateY(-50%); | ||
right: 4px; | ||
} | ||
|
||
.sortArrowUp:after { | ||
content: '▲'; | ||
display: block; | ||
} | ||
|
||
.sortArrowDown:after { | ||
content: '▼'; | ||
display: block; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
export const headerCell: string; | ||
export const sortable: string; | ||
export const sortArrow: string; | ||
export const sortArrowUp: string; | ||
export const sortArrowDown: string; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
import * as registerSuite from 'intern/lib/interfaces/object'; | ||
import { assert } from 'chai'; | ||
import { VNode } from '@dojo/interfaces/vdom'; | ||
import HeaderCell from '../../src/HeaderCell'; | ||
|
||
registerSuite({ | ||
name: 'HeaderCell', | ||
render: { | ||
'renders sortable header cell with descending direction'() { | ||
let clicked = false; | ||
const properties = { | ||
onSortRequest() { clicked = true; }, | ||
column: { | ||
id: 'id', | ||
label: 'foo', | ||
sortable: true | ||
}, | ||
sortDetail: { | ||
columnId: 'id', | ||
descending: true | ||
}, | ||
key: 'id' | ||
}; | ||
const headerCell = new HeaderCell(); | ||
headerCell.setProperties(properties); | ||
|
||
const vnode = <VNode> headerCell.__render__(); | ||
vnode.properties!.onclick!.call(headerCell); | ||
|
||
assert.strictEqual(vnode.vnodeSelector, 'th'); | ||
assert.isFunction(vnode.properties!.onclick); | ||
assert.isTrue(clicked); | ||
assert.equal(vnode.properties!['role'], 'columnheader'); | ||
assert.lengthOf(vnode.children, 2); | ||
assert.equal(vnode.children![0].vnodeSelector, 'span'); | ||
assert.equal(vnode.children![0].text, 'foo'); | ||
assert.equal(vnode.children![1].vnodeSelector, 'div'); | ||
assert.equal(vnode.children![1].properties!['role'], 'presentation'); | ||
}, | ||
'renders sortable header cell with ascending direction'() { | ||
let clicked = false; | ||
const properties = { | ||
onSortRequest() { clicked = true; }, | ||
column: { | ||
id: 'id', | ||
label: 'foo', | ||
sortable: true | ||
}, | ||
sortDetail: { | ||
columnId: 'id', | ||
descending: false | ||
}, | ||
key: 'id' | ||
}; | ||
const headerCell = new HeaderCell(); | ||
headerCell.setProperties(properties); | ||
|
||
const vnode = <VNode> headerCell.__render__(); | ||
vnode.properties!.onclick!.call(headerCell); | ||
|
||
assert.strictEqual(vnode.vnodeSelector, 'th'); | ||
assert.isFunction(vnode.properties!.onclick); | ||
assert.isTrue(clicked); | ||
assert.equal(vnode.properties!['role'], 'columnheader'); | ||
assert.lengthOf(vnode.children, 2); | ||
assert.equal(vnode.children![0].vnodeSelector, 'span'); | ||
assert.equal(vnode.children![0].text, 'foo'); | ||
assert.equal(vnode.children![1].vnodeSelector, 'div'); | ||
assert.equal(vnode.children![1].properties!['role'], 'presentation'); | ||
} | ||
} | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
import './HeaderCell'; | ||
import './Cell'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import order
https://github.com/dojo/meta/blob/master/STYLE.md#ordering
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed