Skip to content
This repository has been archived by the owner on Dec 12, 2018. It is now read-only.

Header cell with unit tests, styling, and new interfaces #8

Merged
merged 8 commits into from
Mar 14, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions src/HeaderCell.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import WidgetBase from '@dojo/widget-core/WidgetBase';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

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 { }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ } => {}

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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'))) {
Copy link
Member

Choose a reason for hiding this comment

The 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,
Copy link
Contributor

Choose a reason for hiding this comment

The 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
sortDetail && sortDetail.descending ? headerCellClasses.sortArrowDown : headerClasses.sortArrowUp

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here re building up the classes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

}) : null
]);
}
}

export default HeaderCell;
13 changes: 13 additions & 0 deletions src/interfaces.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
export interface SortDetails {
columnId: string;
descending?: boolean;
}

export interface ItemProperties<T> {
id: string;
data: T;
Expand All @@ -14,6 +19,14 @@ export interface HasColumn {
column: Column<any>;
}

export interface HasSortDetail {
sortDetail: SortDetails;
}

export interface HasSortEvent {
onSortRequest(sortDetail: SortDetails): void;
}

export interface HasItem {
item: ItemProperties<any>;
}
Expand Down
1 change: 1 addition & 0 deletions src/styles/dgrid.css
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
@import 'headerCell.css';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you're going to offer up a dgrid.css file for people using dgrid outside of our cli-webpack build then you need to ensure you add an exclusion rule for it in your gruntfile otherwise it will be built into a separate css-module and the generated classes will mean zero.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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';
25 changes: 25 additions & 0 deletions src/styles/headerCell.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;
}
5 changes: 5 additions & 0 deletions src/styles/headerCell.css.d.ts
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;
Binary file added src/styles/images/ui-icons_222222_256x240.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
72 changes: 72 additions & 0 deletions tests/unit/HeaderCell.ts
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');
}
}
});
1 change: 1 addition & 0 deletions tests/unit/all.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
import './HeaderCell';
import './Cell';
2 changes: 1 addition & 1 deletion tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
"variable-declaration": "onespace"
} ],
"use-strict": false,
"variable-name": [ true, "check-format", "allow-leading-underscore", "ban-keywords" ],
"variable-name": [ true, "check-format", "allow-leading-underscore", "ban-keywords", "allow-pascal-case" ],
"whitespace": [ true, "check-branch", "check-decl", "check-operator", "check-module", "check-separator", "check-type", "check-typecast" ]
}
}