Conversation
- clean up props / build update
# Conflicts: # dash_table/bundle.js # dash_table/demo.js
| { cached: true, result: lastResult } : | ||
| { cached: false, result: (lastArgs = args) && (lastResult = fn(...args)) }; | ||
| }; | ||
| } |
There was a problem hiding this comment.
A memoize function that knows if the cache has been hit or not
| virtualization: 'fe', | ||
| virtualization_settings: { | ||
| pagination_mode: 'fe', | ||
| pagination_settings: { |
There was a problem hiding this comment.
virtualization made no sense -- this is about pagination, renaming
| derived_viewport_dataframe: [], | ||
| derived_viewport_indices: [], | ||
| derived_virtual_dataframe: [], | ||
| derived_virtual_indices: [], |
There was a problem hiding this comment.
New read-only derived properties
| this.state, | ||
| { setProps, virtualizer } | ||
| virtual.result, | ||
| viewport.result, |
There was a problem hiding this comment.
The ControlledTable is not using the derived values, if someone does not respect the readonly contract, it will not be impacted as it's never used internally
| @@ -0,0 +1,93 @@ | |||
| import { Dataframe, Indices, PaginationMode, PropsWithDefaults, SetProps } from 'dash-table/components/Table/props'; | |||
There was a problem hiding this comment.
Takes props configuration and returns a memoized paginator
| @@ -0,0 +1,78 @@ | |||
| import { Dataframe, Indices, IPaginationSettings, PropsWithDefaults, PaginationMode } from 'dash-table/components/Table/props'; | |||
There was a problem hiding this comment.
Takes virtual dataframe and pagination and returns a memoized viewport dataframe
| @@ -0,0 +1,86 @@ | |||
| import * as R from 'ramda'; | |||
There was a problem hiding this comment.
Takes the dataframe, sorting, filtering and returns a memoized virtual dataframe
|
|
||
| return dataframe.map((datum, virtualIdx) => { | ||
| const realIdx = indices[virtualIdx]; | ||
| return dataframe.map((datum, viewportIdx) => { |
There was a problem hiding this comment.
A lot of naming changes to make usage consistent with the underlying variable
| virtualizer.refresh(); | ||
| const virtual = this.virtualAdapter.get(); | ||
| const viewport = this.viewportAdapter.get(); | ||
| const paginator = this.paginationAdapter.get(); |
There was a problem hiding this comment.
Eval the various adapters for their value and push to derived props and to controlled table
|
@valentijnnieman pr poke! not urgent just yet but would appreciate some feedback so we can start moving this along -- there's a follow up too! tomorrow I will def be pushing this :) |
| return Object.keys(this.props).filter(key => | ||
| !key.startsWith('derived_') && | ||
| (nextProps as any)[key] !== (this.props as any)[key] | ||
| ).length > 0; |
There was a problem hiding this comment.
There's no point re-rendering if only the derived props have been updated -- they are not used internally
There was a problem hiding this comment.
Are there any Ramda functions you could use here instead?
There was a problem hiding this comment.
I'll check
| setProps, | ||
| viewport, | ||
| virtual | ||
| } |
There was a problem hiding this comment.
Calculate all derived props outside the controlled table, add to props
| const invalidateSelection = | ||
| (!invalidatedFilter.cached && !invalidatedFilter.first && filtering === 'be') || | ||
| (!invalidatedPagination.cached && !invalidatedPagination.first && pagination_mode === 'be') || | ||
| (!invalidatedSort.cached && !invalidatedSort.first && sorting === 'be'); |
There was a problem hiding this comment.
We want/need to invalidate props (active_cell, selected_cells, selected_rows) if pagination/sorting/filtering is (1) done in BE and (2) has changed since last render -- whatever the selection was will make no sense -- in the future, BE implementations providing a key/hash/id for the dataframe entries could work around this problem and we could modify processing to calculate new values, etc.
| paginator, | ||
| setProps, | ||
| viewport, | ||
| virtual |
There was a problem hiding this comment.
See below in updateDerivedProps -- the controlled table does not reuse the same props as the exposed derived props -- these are meant to be readonly but there is no way to guarantee sanity from inside the component -- user can do whatever, the functionality will not be affected as we never read/use the derived props.
| }); | ||
| private readonly paginator = derivedPaginator(); | ||
| private readonly viewport = derivedViewportDataframe(); | ||
| private readonly virtual = derivedVirtualDataframe(); |
There was a problem hiding this comment.
memoized values
| private readonly paginationCache = memoizeOneWithFlag(pagination => pagination); | ||
| private readonly sortCache = memoizeOneWithFlag(sort => sort); | ||
| private readonly viewportCache = memoizeOneWithFlag(viewport => viewport); | ||
| private readonly virtualCache = memoizeOneWithFlag(virtual => virtual); |
There was a problem hiding this comment.
we are not so much interested in the value itself, just if the value has changed in context
| } | ||
|
|
||
| export type PropsWithDefaults = IProps & IDefaultProps; | ||
| export type PropsWithDefaultsAndDerived = PropsWithDefaults & IDerivedProps; |
There was a problem hiding this comment.
Isolating the derived props from the rest of the props so they don't pollute into the controlled table contract below
| } | ||
| }; | ||
|
|
||
| export default memoizeOneFactory(getter); No newline at end of file |
There was a problem hiding this comment.
Greatly simplified paginator, virtualization adapter implementations -- these are just calculations onto data and are now represented as such -- wrapped in a memoize factory, which just means that each call on the exported function will create a new scoped / memoized function (e.g. two table instances will get two different memoizers)
| // virtual_indices | ||
| const indices = R.map(datum => map.get(datum) as number, dataframe); | ||
|
|
||
| return { dataframe, indices }; |
There was a problem hiding this comment.
dataframe + fe sorting + fe filtering -> virtual dataframe
if both sorting and filtering are in the be, virtual dataframe === dataframe
valentijnnieman
left a comment
There was a problem hiding this comment.
Looks good 💃 did leave a couple of comments / suggestions, but I approved anyway so you won't be blocked. I did notice the copy_paste_test integration test failing on my machine. Looks like copy pasting works however, in an actual Dash app.
CHANGELOG.md
Outdated
| For example, derived_viewport_dataframe is a readonly view based on | ||
| f(dataframe, filtering params, sorting params, pagination params) --> derived_viewport_dataframe | ||
|
|
||
| Derived properties allow the component to expose complex state that can be useful for a Dash Server developper but without introducing dual states, a situation where multiple properties may represent the same state within the component, making it necessary to reconcile them on each prop update. |
There was a problem hiding this comment.
Small typo here - developper instead of developer
|
|
||
| export default class HeaderFactory { | ||
| private static getSorting(columnId: ColumnId, settings: SortSettings): SortDirection { | ||
| private get props() { |
There was a problem hiding this comment.
How do you feel about naming it getProps() instead?
There was a problem hiding this comment.
Same as for setProps getter below. Calling this is simply this.props, not this.props(), the 'get' does not make much sense here.
| return Object.keys(this.props).filter(key => | ||
| !key.startsWith('derived_') && | ||
| (nextProps as any)[key] !== (this.props as any)[key] | ||
| ).length > 0; |
There was a problem hiding this comment.
Are there any Ramda functions you could use here instead?
| private __adapter = memoizeOne( | ||
| () => new VirtualizationAdapter(this) | ||
| ); | ||
| private get setProps() { |
There was a problem hiding this comment.
I guess then here you could do getSetProps(), although maybe that looks weird. I've always liked the convention of naming getters and setters as getSomething() and setSomething() though.
There was a problem hiding this comment.
I'm assuming here that the get keyword here in TS doesn't add anything to the function name itself.
There was a problem hiding this comment.
The getter / setter as defined here is the same as the JS one -- when used, you're not writing this.setProps(), just this.setProps -- other equivalent would be the old school
Object.defineProperty(this, {
get: function() { return ... ; },
...
});
In essence the idea here is for it to look like a class variable, not a function
- fix for IE/Edge
| return R.any(key => | ||
| !DERIVED_REGEX.test(key) && props[key] !== nextProps[key], | ||
| R.keysIn(props) | ||
| ); |
There was a problem hiding this comment.
Updated to use ramda based on @valentijnnieman comment.. many improvements here.. using 'any' instead of filter, so we can exit early.. and I remembered that String.prototype.startsWith is not supported by Edge/IE -- have to use a regex test instead!
|
@valentijnnieman Do you know what copy_paste test failed? |
* 3.1 props refactoring (#106) * refactor virtualization, virtualization settings, and derived props * refactor renaming paging / pagination props * refactor virtual, viewport and pagination into adapters * isolate derived props * build update * fix regression * improve typing * fix test regression * simplify pagination adapter / refactor * lint * clean up unused props * - change factory - clean up props / build update * fix lint * bump version * add dash level props for virtual_dataframe * refactor fp / derived props * derived props * refactor viewport and virtual controlled props * fix fp regression * fix fp regression * refactor controlled table / table fp * controlled table purecomponent * fix test (rebrake it!) * fix selection regression for be paging/sorting/filtering * improve re-renders & controlled props * fix test regressions * update inner-selection fixture * remove useless spy * - fix pr comment - fix for IE/Edge * clean up * 3.0 clean offsets (#110) * refactor virtualization, virtualization settings, and derived props * refactor renaming paging / pagination props * refactor virtual, viewport and pagination into adapters * isolate derived props * build update * fix regression * improve typing * fix test regression * simplify pagination adapter / refactor * lint * clean up unused props * - change factory - clean up props / build update * fix lint * bump version * add dash level props for virtual_dataframe * cleaup offsets * triad validation * - define external facing classes and attributes * fix regression, update build * fix test regression (invalid props) * update test name * refactor fp / derived props * derived props * refactor viewport and virtual controlled props * fix fp regression * fix fp regression * refactor controlled table / table fp * controlled table purecomponent * fix test (rebrake it!) * fix selection regression for be paging/sorting/filtering * improve re-renders & controlled props * fix test regressions * update inner-selection fixture * remove useless spy * - control columns into visible columns - cleanup "hidden" conditional processing * update changelog * clean up header factory * apply style on first controlled table render * typo/merge miss * derived visible columns * - visual tests for hidden columns * rename functions * - fix dropdown styling regression * lint * 3.1 props fixes (#112) * props fixes * update changelog * bump version * filter typing * Update derivedViewport_test.ts Add basic viewport test with |df| > page_size * ☝️ 3 new review / documentation / target apps see discussion in #108 * 🙈 forgot file * 📝 incorporate @cldougl suggestions 🙇 * 3.1 refactor tests (#113) * props fixes * update changelog * bump version * filter typing * - delete unused usage files - restructure tests folder * - separate cypress and visual tests into 2 ci jobs * - build before tests! * add browsers to the node image for visual-test * 💯 add percent examples * 📝 title/example clarification * reformat sizing test app for failing tests (#125) * Removed .only from dash_test.ts * Production build instead of dev 🙈 * Fix failing tests * 3.1 refactor cells rendering (#123) * WIP - memoize cell event handlers as derived values - isolate cell event handlers * wip - attempt to isolate cell logic from input logic for individual datum * wip - celan up cell wrapper - isolate input logic a bit more * further down the rabbit hole.. - separating operation cells from content * fix dropdown navigation regression * fix selected row regression * renaming / restructuring * rename/restructure * - clean up zipping - separate wrappers from styles * rework style/ast cache * clean up * clean up * build update * improve rendering perf * optimize wrappers generation * build config * - fix typing regression - fix rendering perf regression * - fix navigation regression * simplify slightly the derived props / ui * fix copy/paste regressions * clean up wrapper props * clean up * fix regression on conditional dropdowns * wip, fp the headers * fp content, wrappers, labels, indices from header factory * fix regressions * fp the table itself * fix typing and behavior for table fp * fix sorting icon regression * fix regression * regression * fix column name regression with only 1 header row * fix header actions regression * fix style application on mount * fix regression edit cell in n-th page * fix editing on non-first page (continued) * fix test * 3.1 issue118 width behavior (#130) * WIP - memoize cell event handlers as derived values - isolate cell event handlers * wip - attempt to isolate cell logic from input logic for individual datum * wip - celan up cell wrapper - isolate input logic a bit more * further down the rabbit hole.. - separating operation cells from content * fix dropdown navigation regression * fix selected row regression * renaming / restructuring * rename/restructure * - clean up zipping - separate wrappers from styles * rework style/ast cache * clean up * clean up * build update * improve rendering perf * optimize wrappers generation * build config * - fix typing regression - fix rendering perf regression * - fix navigation regression * simplify slightly the derived props / ui * fix copy/paste regressions * clean up wrapper props * clean up * fix regression on conditional dropdowns * wip, fp the headers * fp content, wrappers, labels, indices from header factory * fix regressions * fp the table itself * fix typing and behavior for table fp * fix sorting icon regression * fix regression * regression * fix column name regression with only 1 header row * fix header actions regression * add width percentage support + content_style * fix style application on mount * fix visual regression with empty df * only apply row style when necessary * fix tab test (no offset) * clean up header styling * use dash-* classes * support default column width (override input behavior) * fix regression edit cell in n-th page * fix editing on non-first page (continued) * fix test * fit to content behavior * fix regressions * fix lint * add column width visual tests * fix dropdown minimum size when using default width * sizing examples * black * fix navigation test regression * fix regressions in visual tests * default column width - fix dropdown width eval * default width columns - fix width when first content row is search filter * percy - add delay before screenshot (attempt to fix FF visual tests) * debugging selenium * fix black * debug selenium * debug selenium * fix black * debug selenium * debug selenium * debug selenium * undo all selenium modifications * default column width - filter inputs behave like cell inputs (do not affect width) * - fixed rows+columns height evaluated correctly * remove dead code * remove .only from tests * add data-dash-column to filter cells * styling examples (#117) * 🌈 styling examples examples that represent of the level of customization that we need in dash-table. The examples are implemented with HTML tables to demonstrate the intended behaviour. I’d like to see each of these examples implemented with the `dash_table` syntax. We’ll use these examples as our `dash-table` documentation * ❌ removing black and pylint this keeps tripping us up and I don’t think it’s worth the pain right now. * Backend examples (#119) * 🏭 backend computed data usage examples * 📊 tying it together w a graph * ❓ not sure what `displayed_pages` does? * Dropdown usage examples (#120) * ⬇️ dropdown usage examples * add conditional dropdown example
* 3.1 props refactoring (#106) * refactor virtualization, virtualization settings, and derived props * refactor renaming paging / pagination props * refactor virtual, viewport and pagination into adapters * isolate derived props * build update * fix regression * improve typing * fix test regression * simplify pagination adapter / refactor * lint * clean up unused props * - change factory - clean up props / build update * fix lint * bump version * add dash level props for virtual_dataframe * refactor fp / derived props * derived props * refactor viewport and virtual controlled props * fix fp regression * fix fp regression * refactor controlled table / table fp * controlled table purecomponent * fix test (rebrake it!) * fix selection regression for be paging/sorting/filtering * improve re-renders & controlled props * fix test regressions * update inner-selection fixture * remove useless spy * - fix pr comment - fix for IE/Edge * clean up * 3.0 clean offsets (#110) * refactor virtualization, virtualization settings, and derived props * refactor renaming paging / pagination props * refactor virtual, viewport and pagination into adapters * isolate derived props * build update * fix regression * improve typing * fix test regression * simplify pagination adapter / refactor * lint * clean up unused props * - change factory - clean up props / build update * fix lint * bump version * add dash level props for virtual_dataframe * cleaup offsets * triad validation * - define external facing classes and attributes * fix regression, update build * fix test regression (invalid props) * update test name * refactor fp / derived props * derived props * refactor viewport and virtual controlled props * fix fp regression * fix fp regression * refactor controlled table / table fp * controlled table purecomponent * fix test (rebrake it!) * fix selection regression for be paging/sorting/filtering * improve re-renders & controlled props * fix test regressions * update inner-selection fixture * remove useless spy * - control columns into visible columns - cleanup "hidden" conditional processing * update changelog * clean up header factory * apply style on first controlled table render * typo/merge miss * derived visible columns * - visual tests for hidden columns * rename functions * - fix dropdown styling regression * lint * 3.1 props fixes (#112) * props fixes * update changelog * bump version * filter typing * Update derivedViewport_test.ts Add basic viewport test with |df| > page_size * ☝️ 3 new review / documentation / target apps see discussion in #108 * 🙈 forgot file * 📝 incorporate @cldougl suggestions 🙇 * 3.1 refactor tests (#113) * props fixes * update changelog * bump version * filter typing * - delete unused usage files - restructure tests folder * - separate cypress and visual tests into 2 ci jobs * - build before tests! * add browsers to the node image for visual-test * 💯 add percent examples * 📝 title/example clarification * reformat sizing test app for failing tests (#125) * Removed .only from dash_test.ts * Production build instead of dev 🙈 * Fix failing tests * 3.1 refactor cells rendering (#123) * WIP - memoize cell event handlers as derived values - isolate cell event handlers * wip - attempt to isolate cell logic from input logic for individual datum * wip - celan up cell wrapper - isolate input logic a bit more * further down the rabbit hole.. - separating operation cells from content * fix dropdown navigation regression * fix selected row regression * renaming / restructuring * rename/restructure * - clean up zipping - separate wrappers from styles * rework style/ast cache * clean up * clean up * build update * improve rendering perf * optimize wrappers generation * build config * - fix typing regression - fix rendering perf regression * - fix navigation regression * simplify slightly the derived props / ui * fix copy/paste regressions * clean up wrapper props * clean up * fix regression on conditional dropdowns * wip, fp the headers * fp content, wrappers, labels, indices from header factory * fix regressions * fp the table itself * fix typing and behavior for table fp * fix sorting icon regression * fix regression * regression * fix column name regression with only 1 header row * fix header actions regression * fix style application on mount * fix regression edit cell in n-th page * fix editing on non-first page (continued) * fix test * 3.1 issue118 width behavior (#130) * WIP - memoize cell event handlers as derived values - isolate cell event handlers * wip - attempt to isolate cell logic from input logic for individual datum * wip - celan up cell wrapper - isolate input logic a bit more * further down the rabbit hole.. - separating operation cells from content * fix dropdown navigation regression * fix selected row regression * renaming / restructuring * rename/restructure * - clean up zipping - separate wrappers from styles * rework style/ast cache * clean up * clean up * build update * improve rendering perf * optimize wrappers generation * build config * - fix typing regression - fix rendering perf regression * - fix navigation regression * simplify slightly the derived props / ui * fix copy/paste regressions * clean up wrapper props * clean up * fix regression on conditional dropdowns * wip, fp the headers * fp content, wrappers, labels, indices from header factory * fix regressions * fp the table itself * fix typing and behavior for table fp * fix sorting icon regression * fix regression * regression * fix column name regression with only 1 header row * fix header actions regression * add width percentage support + content_style * fix style application on mount * fix visual regression with empty df * only apply row style when necessary * fix tab test (no offset) * clean up header styling * use dash-* classes * support default column width (override input behavior) * fix regression edit cell in n-th page * fix editing on non-first page (continued) * fix test * fit to content behavior * fix regressions * fix lint * add column width visual tests * fix dropdown minimum size when using default width * sizing examples * black * fix navigation test regression * fix regressions in visual tests * default column width - fix dropdown width eval * default width columns - fix width when first content row is search filter * percy - add delay before screenshot (attempt to fix FF visual tests) * debugging selenium * fix black * debug selenium * debug selenium * fix black * debug selenium * debug selenium * debug selenium * undo all selenium modifications * default column width - filter inputs behave like cell inputs (do not affect width) * - fixed rows+columns height evaluated correctly * remove dead code * remove .only from tests * add data-dash-column to filter cells * styling examples (#117) * 🌈 styling examples examples that represent of the level of customization that we need in dash-table. The examples are implemented with HTML tables to demonstrate the intended behaviour. I’d like to see each of these examples implemented with the `dash_table` syntax. We’ll use these examples as our `dash-table` documentation * ❌ removing black and pylint this keeps tripping us up and I don’t think it’s worth the pain right now. * Backend examples (#119) * 🏭 backend computed data usage examples * 📊 tying it together w a graph * ❓ not sure what `displayed_pages` does? * Dropdown usage examples (#120) * ⬇️ dropdown usage examples * add conditional dropdown example * - additional tests for editable/readonly - fixes for editable/readonly * bump version * remove useless test column * add editable prop to fixtures * update tests to take into account readonly 'rows' column * - refactor column isEditable calculation
Partial refactoring of props related to dataframe / sorting / filtering / pagination.
Provide a more unified dataframe API for the user.
dataframe -> the full array of data
virtual_dataframe -> df + sorting + filtering
viewport_dataframe -> virtual_df + pagination
This PR does not include BE/FE combination validation, this will come later, but some combinations will cause df, virtual and viewport to have the same values. That's perfectly fine.
Exposing a serie of props prefixed with 'derived_'. These props are readonly and represent derived states of the dataframe that can be calculated from 'first-class' props -- these are used to provided additional insight into the table without muddling the data contract with additional props that represent similar concepts as existing ones, which if all read/write would cause syncing and general sanity issues within the table.
Also cleaning up a few props that are not used anymore.
Note: This PR will introduce a new branch 'develop' -- as these changes might cause instability, this will isolate the changes from the master branch and the existing demo applications until everything is more stable.
Note: Bumping the minor version as the overall changes will be significant either to the internal working of the table and to the less codified portions of the component API/contract.