-
-
Notifications
You must be signed in to change notification settings - Fork 74
3.1 props fixes #112
3.1 props fixes #112
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ export default class Table extends Component<PropsWithDefaultsAndDerived> { | |
super(props); | ||
|
||
this.controlled = this.getControlledProps(this.props); | ||
this.updateDerivedProps(); | ||
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. Make sure the derived props are calculated prior to first render |
||
} | ||
|
||
componentWillReceiveProps(nextProps: PropsWithDefaultsAndDerived) { | ||
|
@@ -84,7 +85,7 @@ export default class Table extends Component<PropsWithDefaultsAndDerived> { | |
pagination_mode, | ||
pagination_settings, | ||
setProps, | ||
viewport.dataframe | ||
virtual.dataframe | ||
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. The paginator should have been using the virtual_df all along |
||
); | ||
|
||
const visibleColumns = this.visibleColumns(columns); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,11 @@ export enum FilteringType { | |
Basic = 'basic' | ||
} | ||
|
||
export interface IDerivedDataframe { | ||
dataframe: Dataframe; | ||
indices: Indices; | ||
} | ||
|
||
export type ActiveCell = CellCoordinates | []; | ||
export type CellCoordinates = [number, number]; | ||
export type ColumnId = string | number; | ||
|
@@ -142,15 +147,9 @@ export type ControlledTableProps = PropsWithDefaults & { | |
setProps: SetProps; | ||
|
||
columns: VisibleColumns; | ||
paginator: IPaginator | ||
viewport: { | ||
dataframe: Dataframe, | ||
indices: Indices | ||
}, | ||
virtual: { | ||
dataframe: Dataframe, | ||
indices: Indices | ||
} | ||
paginator: IPaginator; | ||
viewport: IDerivedDataframe; | ||
virtual: IDerivedDataframe; | ||
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. Iterative typing improvements |
||
}; | ||
|
||
export interface ICellFactoryOptions { | ||
|
@@ -173,9 +172,5 @@ export interface ICellFactoryOptions { | |
selected_cell: SelectedCells; | ||
selected_rows: number[]; | ||
setProps: SetProps; | ||
|
||
viewport: { | ||
dataframe: Dataframe, | ||
indices: Indices | ||
}; | ||
viewport: IDerivedDataframe; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
import * as R from 'ramda'; | ||
|
||
import { memoizeOneFactory } from 'core/memoizer'; | ||
|
||
import { | ||
|
@@ -35,25 +37,31 @@ function getBackEndPagination( | |
function getFrontEndPagination( | ||
pagination_settings: IPaginationSettings, | ||
setProps: SetProps, | ||
virtual_dataframe: Dataframe | ||
dataframe: Dataframe | ||
) { | ||
return { | ||
loadNext: () => { | ||
let maxPageIndex = Math.floor(virtual_dataframe.length / pagination_settings.page_size); | ||
let maxPageIndex = Math.floor(dataframe.length / pagination_settings.page_size); | ||
|
||
if (pagination_settings.current_page >= maxPageIndex) { | ||
return; | ||
} | ||
|
||
pagination_settings.current_page++; | ||
pagination_settings = R.merge(pagination_settings, { | ||
current_page: pagination_settings.current_page + 1 | ||
}); | ||
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. pagination_settings is immutable, create a new one and setprops! -- otherwise render is not triggered on table |
||
|
||
setProps({ pagination_settings }); | ||
}, | ||
loadPrevious: () => { | ||
if (pagination_settings.current_page <= 0) { | ||
return; | ||
} | ||
|
||
pagination_settings.current_page--; | ||
pagination_settings = R.merge(pagination_settings, { | ||
current_page: pagination_settings.current_page - 1 | ||
}); | ||
|
||
setProps({ pagination_settings }); | ||
} | ||
}; | ||
|
@@ -70,14 +78,14 @@ const getter = ( | |
pagination_mode: PaginationMode, | ||
pagination_settings: IPaginationSettings, | ||
setProps: SetProps, | ||
virtual_dataframe: Dataframe | ||
dataframe: Dataframe | ||
): IPaginator => { | ||
switch (pagination_mode) { | ||
case false: | ||
return getNoPagination(); | ||
case true: | ||
case 'fe': | ||
return getFrontEndPagination(pagination_settings, setProps, virtual_dataframe); | ||
return getFrontEndPagination(pagination_settings, setProps, dataframe); | ||
case 'be': | ||
return getBackEndPagination(pagination_settings, setProps); | ||
default: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
import * as R from 'ramda'; | ||
|
||
import derivedViewportDataframe from 'dash-table/derived/viewportDataframe'; | ||
|
||
describe.only('derived viewport', () => { | ||
const viewportDataframe = derivedViewportDataframe(); | ||
|
||
describe('virtual dataframe <= page size', () => { | ||
describe('with no pagination', () => { | ||
it('returns entire dataframe', () => { | ||
const result = viewportDataframe( | ||
false, | ||
{ displayed_pages: 1, current_page: 0, page_size: 250 }, | ||
R.map(() => { }, R.range(0, 5)), | ||
R.range(0, 5) | ||
); | ||
|
||
expect(result.dataframe.length).to.equal(5); | ||
expect(result.indices.length).to.equal(5); | ||
}); | ||
}); | ||
|
||
describe('with fe pagination', () => { | ||
it('returns entire dataframe', () => { | ||
const result = viewportDataframe( | ||
'fe', | ||
{ displayed_pages: 1, current_page: 0, page_size: 250 }, | ||
R.map(() => { }, R.range(0, 5)), | ||
R.range(0, 5) | ||
); | ||
|
||
expect(result.dataframe.length).to.equal(5); | ||
expect(result.indices.length).to.equal(5); | ||
}); | ||
}); | ||
|
||
describe('with be pagination', () => { | ||
it('returns entire dataframe', () => { | ||
const result = viewportDataframe( | ||
'be', | ||
{ displayed_pages: 1, current_page: 0, page_size: 250 }, | ||
R.map(() => { }, R.range(0, 5)), | ||
R.range(0, 5) | ||
); | ||
|
||
expect(result.dataframe.length).to.equal(5); | ||
expect(result.indices.length).to.equal(5); | ||
}); | ||
}); | ||
}); | ||
}); |
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.
turns out a map behaves just like a normal object.. using a number as a key causes a bunch of weird behaviors.. coerce into a string first!
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.
Hmm interesting! What kind of behaviours did you run into? Maybe a silly question, but if you're using a string here, could you not use a regular Object instead of a Map?