-
Notifications
You must be signed in to change notification settings - Fork 12
Customizable Headers and Footers, fixes #11 #38
Conversation
Codecov Report
@@ Coverage Diff @@
## master #38 +/- ##
==========================================
+ Coverage 98.55% 98.69% +0.13%
==========================================
Files 12 13 +1
Lines 277 306 +29
Branches 49 55 +6
==========================================
+ Hits 273 302 +29
Partials 4 4
Continue to review full report at Codecov.
|
src/ColumnHeaders.ts
Outdated
import { v, w } from '@dojo/widget-core/d'; | ||
import { DNode } from '@dojo/widget-core/interfaces'; | ||
import { RegistryMixin, RegistryMixinProperties } from '@dojo/widget-core/mixins/Registry'; | ||
import { ThemeableMixin, theme, ThemeableProperties } from '@dojo/widget-core/mixins/Themeable'; |
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.
theme
should be first
src/Grid.ts
Outdated
@@ -25,6 +27,8 @@ export const GridBase = ThemeableMixin(RegistryMixin(WidgetBase)); | |||
export interface GridProperties extends ThemeableProperties, HasColumns { | |||
registry?: GridRegistry; | |||
dataProvider: DataProviderBase; | |||
footers?: Array<HeaderType | DNode>; |
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.
generally we use (HeaderType | DNode)[]
src/Grid.ts
Outdated
onSortRequest | ||
}), | ||
theme | ||
}, <DNode[]> headers.map((child) => { |
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.
Seems odd we need a cast here. Also this logic is quite hard to follow in line and is duplicated below. This should probably be broken into a function that is easier to grok and maintain.
Type: feature
The following has been addressed in the PR:
Description: