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

Customize cell content, fixes #12 #37

Merged
merged 4 commits into from
Jun 6, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 4 additions & 4 deletions src/Cell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ import WidgetBase from '@dojo/widget-core/WidgetBase';
import { RegistryMixin, RegistryMixinProperties } from '@dojo/widget-core/mixins/Registry';
import { v } from '@dojo/widget-core/d';
import { DNode } from '@dojo/widget-core/interfaces';
import { HasValue, HasColumn, HasItem } from './interfaces';
import { HasColumn, HasContent, HasItem, HasValue } from './interfaces';
import { theme, ThemeableMixin, ThemeableProperties } from '@dojo/widget-core/mixins/Themeable';

import * as cellCss from './styles/shared/cell.m.css';
import * as css from './styles/cell.m.css';

export interface CellProperties extends HasColumn, HasItem, HasValue, RegistryMixinProperties, ThemeableProperties {
export interface CellProperties extends HasColumn, HasContent, HasItem, HasValue, RegistryMixinProperties, ThemeableProperties {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of HasValue for Cell? It's not used in the rendering... is it envisioned that one might want to read the value of value from a Cell instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's my thought, yes.

key: string;
}

Expand All @@ -19,14 +19,14 @@ export const CellBase = ThemeableMixin(RegistryMixin(WidgetBase));
class Cell extends CellBase<CellProperties> {
render(): DNode {
const {
value = ''
content
} = this.properties;

return v('td', {
role: 'gridcell',
classes: this.classes(cellCss.cell, css.rowCell)
}, [
String(value)
content
]);
}
}
Expand Down
31 changes: 28 additions & 3 deletions src/Row.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import * as tableCss from './styles/shared/table.m.css';
export const RowBase = ThemeableMixin(RegistryMixin(WidgetBase));

export interface RowProperties extends WidgetProperties, HasColumns, RegistryMixinProperties, ThemeableProperties {
item: ItemProperties<any>;
item: ItemProperties;
key: string;
}

Expand All @@ -37,15 +37,40 @@ class Row extends RowBase<RowProperties> {
classes: this.classes(tableCss.table, css.rowTable)
}, [
v('tr', columns.map((column) => {
const { id, field } = column;
const { field, id } = column;

let value: any;
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps would be nicer it could be simplified a bit (does assume consolidating the two render functions):

let value = item.data[ field || id] ? String(item.data[ field || id ]) : '';
if (column.get) {
    value = column.get(item, column);
}

let content: DNode = value;
if (column.render) {
    content = column.render(item, column, value);
}

if (typeof column.get === 'function') {
// Get the value from the column callback
value = column.get(item, column);
}
else if (typeof column.get !== 'undefined') {
// Get the value from the column property
value = column.get;
}
else {
// Get the value using a property lookup on the item data
value = item.data[ field || id ];
}

let content: DNode;
if (column.render) {
// The column callback calculates its own value and DNode/string
content = column.render({ value, item, column });
}
else {
// The value from get/item data is cast to a string
content = String(value);
}

return w<Cell>('cell', {
content,
column,
item,
key: id,
registry,
theme,
value: item.data[ field || id ]
value
});
}))
])
Expand Down
41 changes: 35 additions & 6 deletions src/interfaces.d.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,50 @@
export interface Column<T> {
import { DNode } from '@dojo/widget-core/interfaces';

/**
* @type Column
*
* Adds information about a column that will be used by the
* grid when retrieving and displaying data.
*
* @typeparam T Used by field property and cell customization functions to enforce type safety
* @typeparam V Used by cell customization to ensure the same type is used by get and render
* @property field Property on the row's data item to use to look up the column's value
* @property get Manually return the value or content to use for this column
* @property id A unique identifier for this column
* @property label The label to use in the column's header
* @property sortable Set to false to indicate the column is not sortable
* @property render Use this column's value to provide content for this column
*/
export interface Column<T = any, V = string> {
field?: keyof T;
get?: V | ((item: ItemProperties<T>, column: Column<T>) => V);
id: string;
label?: string;
sortable?: boolean; // default true
render?(options: ColumnRenderOptions<T, V>): DNode;
}

export interface ColumnRenderOptions<T = any, V = string> {
column: Column<T>;
item: ItemProperties<T>;
value: V;
}

export interface DataProperties<T> {
items: ItemProperties<T>[];
sort?: SortDetails[];
}

export interface HasContent {
content: DNode;
}

export interface HasColumn {
column: Column<any>;
column: Column<any, any>;
}

export interface HasColumns {
columns: Column<any>[];
columns: Column<any, any>[];
}

export interface HasSortDetail {
Expand All @@ -35,18 +64,18 @@ export interface HasSortEvent {
}

export interface HasItem {
item: ItemProperties<any>;
item: ItemProperties;
}

export interface HasItems {
items: ItemProperties<any>[];
items: ItemProperties[];
}

export interface HasValue {
value: string;
}

export interface ItemProperties<T> {
export interface ItemProperties<T = any> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I understanding correctly that dgrid requires data objects to look like this:

{
    id: <idValue>,
    data: {
        <... data properties>
    }
}

Are we not able to continue dgrid 1's design of accepting flatter objects (with support for idProperty as well):

{
    id: <idValue>,
    <... data properties>
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now, the object structure is simple, but ItemProperties adds much more moving forward.

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 it would be very good if dgrid 2 is able to continue supporting data in the same format as dgrid 1 unless there's a compelling reason not to - is there?

While Dojo's data solutions have evolved over the years, the basic format of a data object has been consistent for years. What is the compelling need to change it?

Also - considering dgrid 2 requires a data provider, are my concerns relevant? Can one store be used for consumers that like the old data format, and also for dgrid, with the differences implemented in the data provider?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't really have anything to do with the data - it's what the data provider gives to the grid. ArrayDataProvider still supports simple data with idProperty.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok great!

id: string;
data: T;
}
Expand Down
2 changes: 1 addition & 1 deletion src/providers/ArrayDataProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export interface ArrayDataProviderOptions<T> extends DataProviderOptions {
}

function expand(items: any[], idProperty: string) {
const array: ItemProperties<any>[] = [];
const array: ItemProperties[] = [];
for (const item of items) {
const id = String(item[idProperty]);
array.push({
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/Body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ registerSuite({
},

'Simple item'() {
const columns: Column<any>[] = [
const columns: Column[] = [
{ id: 'foo' },
{ id: 'bar' }
];
const item: ItemProperties<any> = {
const item: ItemProperties = {
id: '1',
data: {
id: 1,
Expand Down
41 changes: 5 additions & 36 deletions tests/unit/Cell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import { Column, ItemProperties } from '../../src/interfaces';
import * as cellCss from '../../src/styles/shared/cell.m.css';
import * as css from '../../src/styles/cell.m.css';

const column: Column<any> = {
const column: Column = {
id: 'column'
};
const item: ItemProperties<any> = {
const item: ItemProperties = {
id: 'item',
data: {}
};
Expand All @@ -28,13 +28,14 @@ registerSuite({
widget.destroy();
},

'Cell value used for child node'() {
'Cell content used for child node'() {
widget.setProperties({
column,
content: 'Hello, World!',
key: column.id,
item,
registry,
value: 'Hello, World!'
value: 'unexpected'
});

widget.expectRender(v('td', {
Expand All @@ -43,37 +44,5 @@ registerSuite({
}, [
'Hello, World!'
]));
},

'Cell value missing'() {
widget.setProperties(<any> {
column,
item,
registry
});

widget.expectRender(v('td', {
role: 'gridcell',
classes: widget.classes(cellCss.cell, css.rowCell)
}, [
''
]));
},

'Cell value is stringified'() {
widget.setProperties({
column,
key: column.id,
item,
registry,
value: <any> 1234
});

widget.expectRender(v('td', {
role: 'gridcell',
classes: widget.classes(cellCss.cell, css.rowCell)
}, [
'1234'
]));
}
});
6 changes: 3 additions & 3 deletions tests/unit/Grid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const compareRegistryProperty: GridRegistry = <any> compareProperty((value) => {
return false;
});

const columns: Column<any>[] = [
const columns: Column[] = [
{ id: 'name', label: 'Name' }
];

Expand All @@ -40,7 +40,7 @@ const items = [
}
];

const itemProperties: ItemProperties<any>[] = [
const itemProperties: ItemProperties[] = [
{
id: '1',
data: items[0]
Expand All @@ -62,7 +62,7 @@ const items2 = [
}
];

const itemProperties2: ItemProperties<any>[] = [
const itemProperties2: ItemProperties[] = [
{
id: '3',
data: items2[0]
Expand Down
Loading