[Docs] Re-organize Data grid docs#5713
Merged
cchaos merged 66 commits intoelastic:mainfrom Mar 15, 2022
Merged
Conversation
* [Setup] Move toolbar CSS to own file * [Cleanup] DRY out repeated specific CSS class - naming was incorrect in the case of the style selector popover * Reorder toolbar per Caroline's mocks - Columns & Sort on left, with additionalControls to the right of those (+ update prop types to indicate that) - Style (TBD naming) and Fullscreen to the right * Convert display buttons to icons only + tweak grid styles icon to match Caroline's mock + popover location * Convert density UI to a compressed EuiFormRow w/ new text/order per mocks - refactor densityOptions to const outside hook * Update test snapshots * Add changelog entry * [PR feedback] Revert location of additionalControls + fix props description * [PR feedback] CSS feedback * [PR feature request] Change toolbar icon based on grid density * [PR feedback][extra] Fix missing sort control on styling example NB: sorting functionality won't actually work/isn't hooked up, but this should at least allow the show/hide UI toggle to work as expected * updoot snaps * Fix changelog * [misc] Reorganize EuiDataGridToolbar utils to bottom of file - makes it easier for devs to immediately jump to the main component/fn, rather than have to scroll past helper/minutae details - if it gets complex enough, also worth pulling out to a separate file * Rename CSS toolbar class to left/right to match API - Per GH discussion display/data is ambiguous and naming this left/right is likely more helpful * Add API support for left/right positioning of `additionalControls` * Update additionalControls documentation + example * PR feedback & cleanup from Caroline (#4) * Update some props comments and fix popover’s responsiveness * [Design] Remove a few extraneous style overrides to control buttons And lower left padding * Fixed add/remove of fullscreen body class * Added tooltips around icon buttons with long delay * Updated examples and snippets with “real world” examples Removed all the unnecessary manual additions of `className="euiDataGrid__controlBtn”` * More responsive fixes * Change class name * Update CSS class to be more specific - It isn't necessary for popovers without drag & drop, name becomes verbose but it's hopefully worth it for clearer usage * closes elastic#5092 - add changelog for fullscreen bugfix in Caroline's work * [PR feedback] Changelog * Update snapshots * PR feedback * Remove unused CSS class - Class was removed in a previous commit by Caroline Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
- so that both the columnSelector and displaySelector can use it
+ update docs: - reorder UI to match toolbar layout - fix legends + add unit tests (& improve some existing unit tests)
NYI: conditional lineCount controls + unit tests
+ fix cell height to dynamically on lineCount change
…efined heights but defined rowHeightsOptions objects e.g. - empty objs, or lineHeight set but nothing else + simplify this.props
…ined mode - Multiline content includes items with `<br>`/`<p>` tags etc that end up incorrectly vertically centered to their increased height
… + density - by giving it the same setRowHeight logic as line counts with a single line + rename recalculateLineCountHeight for clarity now that it's no longer being used for just lineCount
- to make the difference more obvious when switching between single and custom
…to datagrid-row-height-control
- expand buttons on single lines should now be more vertically aligned - The manual button/checkbox changes for control columns is somewhat required for auto height to not be dramatically different for single lines
… rows - when hovering over rows - on striped rows - Not caused by this PR, but this was really annoying me, haha
- Slightly less grody to look at in-code, and allows us to provide more comments for context The more I look at __expandButton the more I dislike the naming also but it would involve too many changes right now to rename button->actions, so I added a TODO
+ Remove ability to set negative or zero numbers, which causes the grid to flip out
- This should have been done in 79c878d, but I missed it
…ions are disabled Notes: - Strongly recommend turning off whitespace changes to reduce indentation noise - This could have been done at the `data_grid_toolbar` level and could be a microperf optimization to do so, but since this is a relatively unlikely edge case in any case I don't think it's worth the overoptimization when this approach is relatively straightforward to implement
…to datagrid-row-height-control
…tion FROM EuiDataGrid type to reduce duplication
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5713/ |
4 tasks
cee-chen
reviewed
Mar 15, 2022
Comment on lines
-14
to
-16
| `${root}#/tabular-content/data-grid-styling-and-control`, | ||
| `${root}#/tabular-content/data-grid-virtualization`, | ||
| `${root}#/tabular-content/data-grid-row-heights-options`, |
Contributor
There was a problem hiding this comment.
🎉 🎉 Excited to get a11y checking in for all our data grid pages! Thank you for this!
cee-chen
reviewed
Mar 15, 2022
- Use react router Link over EuiLink so that URLs work as expected on staging - Fix control columns hash
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5713/ |
…rops + move the exported markdown logic to the new `props/` dir, since they fit there better than `playground/` + change our playground flyouts to use bold props instead of linked props, since those can never really work correctly (actual props tabs/tables will still use links)
- if only markdown props are changing, we can reuse getDescription with passed markdown props
cee-chen
reviewed
Mar 15, 2022
cee-chen
reviewed
Mar 15, 2022
cee-chen
reviewed
Mar 15, 2022
cee-chen
reviewed
Mar 15, 2022
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5713/ |
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
cchaos
commented
Mar 15, 2022
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
- for whatever reason, passing undefined for some props is causing cell height to be NaN. Using default fallbacks for the base code fixes the issue
Contributor
Author
|
I think all the other comments have been addressed. |
cee-chen
approved these changes
Mar 15, 2022
Contributor
cee-chen
left a comment
cee-chen
left a comment
There was a problem hiding this comment.
OK, that's all my feedback!! 🎉 This is amazing, thanks so much again for all this wonderfulness!!
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5713/ |
cee-chen
added a commit
to cee-chen/eui
that referenced
this pull request
Mar 18, 2022
…ng` to undefined - rather than just omitting it - see elastic#5713 (comment)
cee-chen
pushed a commit
that referenced
this pull request
Mar 18, 2022
…p not correctly triggering grid rerenders (#5712) * [REVERT] Test rowHeightsOptions props * Force a rerender whenever rowHeightsOptions (that affect row heights) change * Misc cleanup - move rowHeightUtils.setRerenderGridBody to same location as rowHeightUtils.setGrid - move inline comment to where fn is being invoked - update useRowHeightUtils hook comment * Work around failing data_grid.test.tsx tests - not totally sure why this requestAnimationFrame is causing issues but not others * Add changelog entry * changelog * Add useRowHeightUtils unit tests * [misc fix] Handle consumers specifically setting `gridStyle.cellPadding` to undefined - rather than just omitting it - see #5713 (comment) * Revert demo example
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Don't mind the commit messages, I had started this branch off an old PR
Consolidated props comments & snippets
Under the old structure there was a section called 'Snippet with every feature in use' which contained even more descriptions/comments about use cases either specific to the prop or to the example snippet. I've moved any comments directly related to the prop itself to the
data_grid_types.tsfile and created an object of snippets.This way I am now displaying a props "table" of all the high-level props, pulling the props-docs directly from their type and then finding a matching snippet and link out to more docs relevant to that prop.
For the snippets, any place that was directly displaying snippets in the example intros has been moved to the snippet tab.
New outline
I've shifted the "Tabular content" down a few side nav sections and consolidated the data grid docs into the following outline:
Docs utilities
In order to support this PR, I also added a couple docs/props utilies:
getPropsFromComponent; And replaced theme docs'getPropsFromThemeValueversiongetDescriptionandgetDescriptionSmallwhich returns a MarkdownFormat with bolded #referencesChecklist
[ ] Added or updated jest and cypress tests[ ] Checked for breaking changes and labeled appropriately[ ] A changelog entry exists and is marked appropriately