Conversation
- hidden vs. visible columns props at sanitation
# Conflicts: # src/dash-table/components/FilterFactory.tsx # src/dash-table/components/HeaderFactory.tsx # src/dash-table/components/Table/props.ts # src/dash-table/derived/filter/map.ts # src/dash-table/derived/header/content.tsx
| Clearable = 'clearable', | ||
| ClearableMerged = 'clearableMerged', | ||
| Actionable = 'actionable', | ||
| ActionableMerged = 'actionableMerged', |
There was a problem hiding this comment.
Modified clearable modes to display all actions instead
Similar props are: clearable, deletable, hideable, renamable. All of them now have
|
- update docstrings
| }</label> | ||
| </div>) | ||
| }; | ||
| }))).filter(i => !R.isNil(i)).sort((a, b) => a.j - b.j).sort((a, b) => a.i - b.i).map(a => a.component)} |
There was a problem hiding this comment.
This is a bit messy -- can rework if seen as problematic.
Generates a sparse component[][] containing the list of inputs but keeping their associated index, remove nil items and sort in reverse order of condition/priority (columns are more important than rows -- ordering is like a radix sort), retrieve the component from the wrapper.
There was a problem hiding this comment.
.sort((a, b) => (a.i - b.i) || (a.j - b.j)) ?
There was a problem hiding this comment.
Right. That should work too. Checking / updating.
| fill_width?: boolean; | ||
| filter_query?: string; | ||
| filter_action?: TableAction; | ||
| hideable?: boolean | boolean[]; |
| fill_width: boolean; | ||
| filter_query: string; | ||
| filter_action: TableAction; | ||
| hideable: boolean | boolean[]; |
| i === 0 : | ||
| typeof flag === 'boolean' ? | ||
| flag : | ||
| !!flag && flag[i]; No newline at end of file |
There was a problem hiding this comment.
This piece of logic is needed in lots of places now
| * which column header row to display the `rename` action button in by | ||
| * supplying an array of booleans. | ||
| * For example, `[true, false]` will display the `rename` action button | ||
| * on the first row, but not the second row. |
There was a problem hiding this comment.
Are we using Python (True) or JS (true) capitalization here? This inconsistency exists all sorts of places across our component repos, but IIRC we said we'd try to standardize on JS in the JS code, paving the way for language-dependent translation at some future time in the back-end codegen.
Also [false, ..., true] is ambiguous. Perhaps [false, ..., false, true]? That said I might just remove the shortcut sentence, as it's not a fixed shortcut (depends on # of headers) and I think it's already pretty clear without that.
There was a problem hiding this comment.
Will improve consistency for the modified props and go the JS route. Good point for the array examples.
alexcjohnson
left a comment
There was a problem hiding this comment.
Looks great! Last doc comment is nonblocking. 💃
- improve boolean array usage examples
Hidden columns feature #314
Support to toggle columns visibility.
API modifications
column.hideable: boolean | undefinedhidden_columns: string[] | undefinedThis proposed solution deviates from the one describe in the parent issue in a few ways.
hideableonly supportsboolean: supportingboolean[]makes it hard to give a consistent experience when toggling visibility while supporting making non-hideable but hidden columns visible if necessary. It also requires more complex UI interactions that are not obvious to define correctly.. also deviates from more simple column visibility ideas usually associated with spreadsheetsSo,
hideactionWhile not ideal this solution should be flexible enough to make it possible for us to improve upon while not limiting our options.
On a totally different note, with the addition of multiple actions in the header cells, the use of unicode characters is starting to be problematic as they hardly ever align correctly or feel consistent. With that in mind, introducing
fortawesome, a JS / es6-aware (read: tree shaking) wrapper around Font-Awesome free icon library to be used for the actions.One of the goal of a previous PR was to make it easy for users to override the default icon with an icon of their choice. This is still possible with the proposed solution, but the selector(s) will be slightly different. As (a) the modified code w/ customization support has not been release yet and (b) this must be done through unofficial
cssprops or custom stylesheet, this seems fine.