-
Notifications
You must be signed in to change notification settings - Fork 75
new(DataTable): Add xLarge row size. Fix forceUpdate logic. #367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
35b9cd9
00dd92c
9e478ba
1376e36
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 |
|---|---|---|
|
|
@@ -66,6 +66,8 @@ export class DataTable extends React.Component<DataTableProps & WithStylesProps, | |
| sortDirection: this.props.sortDirectionOverride!, | ||
| }; | ||
|
|
||
| timeoutId: number = 0; | ||
|
|
||
| constructor(props: DataTableProps & WithStylesProps) { | ||
| super(props); | ||
| if (this.props.dataTableRef) { | ||
|
|
@@ -114,10 +116,7 @@ export class DataTable extends React.Component<DataTableProps & WithStylesProps, | |
| const { dynamicRowHeight, data, filterData, width, height, sortByCacheKey } = this.props; | ||
| const { sortBy, sortDirection } = this.state; | ||
| const dimensionsChanged = width !== prevProps.width || height !== prevProps.height; | ||
| const sortChanged = | ||
| sortBy !== prevState.sortBy || | ||
| sortDirection !== prevState.sortDirection || | ||
| sortByCacheKey !== prevProps.sortByCacheKey; | ||
| const sortChanged = sortByCacheKey !== prevProps.sortByCacheKey; | ||
| const sortedData: IndexedParentRow[] = this.getData( | ||
| data!, | ||
| sortBy, | ||
|
|
@@ -134,20 +133,19 @@ export class DataTable extends React.Component<DataTableProps & WithStylesProps, | |
| x.metadata.originalIndex !== oldFilteredData[i].metadata.originalIndex, | ||
| )); | ||
|
|
||
| if (dynamicRowHeight && (filteredDataChanged || dimensionsChanged || sortChanged)) { | ||
| // We need to make sure the cache is cleared before React tries to re-render. | ||
| window.setTimeout(() => { | ||
| this.cache.clearAll(); | ||
| this.forceUpdate(); | ||
| }, 0); | ||
| } | ||
|
|
||
| if (this.props.data !== prevProps.data) { | ||
| this.keys = getKeys(this.props.keys!, this.props.data!); | ||
|
|
||
| this.setState({ | ||
| expandedRows: new Set(), | ||
| }); | ||
| } else if (dynamicRowHeight && (filteredDataChanged || dimensionsChanged || sortChanged)) { | ||
| // We need to make sure the cache is cleared before React tries to re-render. | ||
| if (this.timeoutId) window.clearTimeout(this.timeoutId); | ||
| this.timeoutId = window.setTimeout(() => { | ||
| this.cache.clearAll(); | ||
| this.forceUpdate(); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -224,10 +222,11 @@ export class DataTable extends React.Component<DataTableProps & WithStylesProps, | |
|
|
||
| if (dynamicRowHeight) { | ||
| // We need to make sure the cache is cleared before React tries to re-render. | ||
| window.setTimeout(() => { | ||
| if (this.timeoutId) window.clearTimeout(this.timeoutId); | ||
|
Contributor
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 a timer is already set, shouldn't that one just run instead of clearing and creating a new one? |
||
| this.timeoutId = window.setTimeout(() => { | ||
| this.cache.clearAll(); | ||
| this.forceUpdate(); | ||
| }, 0); | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { WidthProperties } from './types'; | ||
| import { WidthProperties, RowHeightOptions } from './types'; | ||
|
|
||
| export const SELECTION_OPTIONS = { | ||
| ACTIVE: 'ACTIVE', | ||
|
|
@@ -13,14 +13,15 @@ export const STATUS_OPTIONS = { | |
| }; | ||
|
|
||
| type HeightMap = { | ||
| [key: string]: number; | ||
| [key in RowHeightOptions]: number; | ||
|
Contributor
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. nice |
||
| }; | ||
|
|
||
| export const HEIGHT_TO_PX: HeightMap = { | ||
| micro: 32, | ||
| small: 48, | ||
| regular: 56, | ||
| large: 64, | ||
| xLarge: 80, | ||
| jumbo: 108, | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ import { DataTable } from './DataTable'; | |
|
|
||
| export type DataTableRef = (instance: DataTable) => void; | ||
| export type TableRef = React.RefObject<Table>; | ||
| export type RowHeightOptions = string; | ||
| export type RowHeightOptions = 'micro' | 'small' | 'regular' | 'large' | 'xLarge' | 'jumbo'; | ||
|
Contributor
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. Lets just call it |
||
| export type HeightOptions = RowHeightOptions | undefined; | ||
| export type ColumnLabelCase = 'sentence' | 'title' | 'uppercase' | ''; | ||
|
|
||
|
|
@@ -45,6 +45,8 @@ export interface DataTableProps { | |
| height?: number; | ||
| /** References row fields to render as columns, infered from data if not specified. */ | ||
| keys?: string[]; | ||
| /** If dynamicRowHeight is enabled, this sets the maximum value for measured row height. */ | ||
| maximumDynamicRowHeight?: number; | ||
| /** If dynamicRowHeight is enabled, this sets the minimum value for measured row height. */ | ||
| minimumDynamicRowHeight?: number; | ||
| /** | ||
|
|
||
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.
we already invoke
this.cache.clearAll();when updatingsortByorsortByDirection, so this was doing a double update