Issue 280 - Refactor cell content #281
Conversation
…lization-2 # Conflicts: # CHANGELOG.md # dash_table/package-info.json # demo/App.js # demo/data.ts # package-lock.json # package.json # src/core/components/IsolatedInput/index.tsx # tests/cypress/tests/standalone/navigation_test.ts
- fix input/label regression
…nto 3.1-rework-cell-content # Conflicts: # dash_table/bundle.js # dash_table/demo.js
…tent # Conflicts: # CHANGELOG.md # dash_table/bundle.js # dash_table/demo.js # src/dash-table/derived/cell/inputs.tsx
| value: any; | ||
| } | ||
|
|
||
| export default class CellDropdown extends PureComponent<IProps> { |
There was a problem hiding this comment.
Standalone Dropdown component pulled out of pre-existing CellInput comp
There was a problem hiding this comment.
Well it still uses react-select in the end :(
| conditionalDropdowns: [], | ||
| type: ColumnType.Text | ||
| }; | ||
| export default class CellInput extends PureComponent<ICellProps, ICellState> { |
There was a problem hiding this comment.
Standalone Input component -- what's left after removing Label and Dropdown from the existing implementation
| value: any; | ||
| } | ||
|
|
||
| export default class CellLabel extends PureComponent<IProps> { |
There was a problem hiding this comment.
Standalone Label component pulled out of pre-existing CellInput comp -- as before we use this to display non-editable fields, fallback dropdown fields and to optimize inputs while cell is not active.
| default: | ||
| return CellType.Label; | ||
| } | ||
| } |
There was a problem hiding this comment.
What type of cell content
| ...['dash-cell-value'] | ||
| ].join(' '); | ||
|
|
||
| switch (getCellType(active, isEditable, dropdown, column.type)) { |
There was a problem hiding this comment.
Once the type of cell content is decided, apply the right one
| value: any; | ||
| } | ||
|
|
||
| export default class CellDropdown extends PureComponent<IProps> { |
| const dropdown = this.refs.dropdown as any; | ||
|
|
||
| if (dropdown && document.activeElement !== dropdown) { | ||
| // Limitation. If React >= 16 --> Use React.createRef instead to pass parent ref to child |
There was a problem hiding this comment.
Would it not be possible to pass a ref down from the parent, regardless of React >= 16?
There was a problem hiding this comment.
This is what I have in mind: ref forwarding (added in React 16.3)
https://reactjs.org/docs/forwarding-refs.html
There was a problem hiding this comment.
Ah gotcha. Yeah that would be nice!
| case ColumnType.Numeric: | ||
| return (!active || !editable) ? CellType.Label : CellType.Input; | ||
| case ColumnType.Dropdown: | ||
| return (!dropdown || !editable) ? CellType.Label : CellType.Dropdown; |
There was a problem hiding this comment.
Because dropdown is of type any, it's not immediately obvious what this is (for me at least). As I'm reading it, I'd think that if case ColumnType.Dropdown, then dropdown should be true too, in other words, when would the type be ColumnType.Dropdown but dropdown would be false?
There was a problem hiding this comment.
Good point. I'll see if I can tighten the typing here
There was a problem hiding this comment.
Improved typing so we know we have either dropdown values (array) or undefined -- there were two sets of independent interfaces used to define dropdown labels and values -- removed one, kept the other. Lots of small additional changes all over the place for that consolidation.
Closes #280 in preparation for #166.
Breaks up the existing CellInput component into all its flavors (label, input, dropdown) and moving the choice into the derived/cell/contents calculation. Separating the logic from the flavors implementation details will make it easier to support the additional data types and scenarios that pop up in the data types epic.
This is all prep work for the data types. There's no additional feature here -- additional sanity may be needed but the tests cover a pretty broad range of cases already.
Not rebasing as there are not that many changes despite the large amount of commits (this branch is a child of the virtualization branch -- that's why!)