[Docs EuiTable] Replace all the demos using data_store with faker-js
#6521
Conversation
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
elizabetdev
added
documentation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6521/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6521/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6521/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6521/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6521/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6521/ |
- provide correct EuiBasicTable typescript usages/definitions
+ remove it entirely from `footer` and `expanding_rows` examples - those demos really don't need that behavior at all
- not being used in multiple places, so no need to DRY it out
+ DRY out `renderStatus` - not totally clear to me why those examples have such complex mobile examples + add a `mobileOptions.only` example in the `mobile` demo to make up for removed examples
- there's no sorting being passed to the table, so it does nothing
- remove need for randomly generated group IDs - simplify var names - use value passed directly from `onChange` instead of using a `.find()` - use flex group for button toggles instead of ` `s
- move `getEmojiFlag` util to bottom of the file - it's not relevant to anything table-related and shouldn't be the first thing a dev has to read/parse - remove unnecessary extra flag/countryCode() call
- if `userLength` isn't being used anywhere else, just hard code it in - no need for a var - remove unnecessary `filteredUsers` logic + remove unnecessary `RIGHT_ALIGNMENT` import
- move `columns` out to static init where logical - add headings to sort out manual pagination/sorting logic where it isn't necessarily relevant to the demo (but aids in visual output) - add comment to `findUsers` to more clearly explain what it's doing - remove unnecessary abstractions for longer files
Contributor
|
@miukimiu I did a type and general code cleanup/organization pass - this is looking good to me at this point, although it's getting a bit late here so I didn't end up doing a final QA sweep - would super appreciate it if you wouldn't mind quickly re-checking staging! FWIW, I'm 50/50 on this, but I think we should consider dropping the emoji flag logic. It's adding an annoying codesandbox "problem" (hoisting is legit, codesandbox!) and the initialization of it is IMO distracting for devs to parse when they should be focusing on the actual table component code. Would you be open to changing the |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6521/ |
Contributor
Author
|
Thanks, @cee-chen. I'll do another review and I'll change the country logic. |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6521/ |
Contributor
Author
I tested again the staging and there's only one demo "Adding a footer to a table" that gives an error in CodeSandbox: It only happens in that example and I believe it is related to one of the dependencies All the rest look good to me. Thanks for the help! 👍🏽 |
Contributor
Ooh awesome catch, I was debating removing that one too tbh. I think we should just nuke it and print out the non-unique number of things - any objections? |
Contributor
Author
|
@cee-chen, no objections. 👍🏽 |
- replace one instance with a `Set` example for uniqueness
- this column is now fairly long compared to before - let's restore the previous table visual apperances by truncating
Contributor
|
@miukimiu not a blocker / not related to this PR as I think this is already happening on prod, but do you have any idea why selected rows appear to be changing height by like 1-2 px? I believe it's the disabled action buttons as it doesn't happen for the basic example. This is the expanded rows demo FWIW
|
- it doesn't really make sense to have a last name be a link as opposed to a username - some demos were doing this, some weren't, so standardize the rendering - remove actual external links to github - most of the newly random usernames aren't valid in any case
cee-chen
approved these changes
Jan 19, 2023
Contributor
cee-chen
left a comment
cee-chen
left a comment
There was a problem hiding this comment.
Alrighty, this is looking really good to me at this point! Thank you for taking the time to improve the dev experience of our tables docs Elizabet, I think people will really appreciate it down the road!! (especially having references to correctly typed columns etc - that always tripped me up in Kibana)
I'm looking into the line height issue I mentioned in a separate comment and I think I know what's happening (it's the tooltip). I'll investigate a fix in a separate PR as that will likely affect source code.
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6521/ |
Contributor
Author
|
Thanks for all the help, @cee-chen! 🥳 |
cee-chen
added a commit
that referenced
this pull request
Jan 24, 2023
* [EuiToolTip] Add `repositionOnScroll` prop (#6515) * Add `repositionOnScroll` prop and event listener * [Docs] Add fixed tooltip example + add auto-focus behavior for keyboard users * changelog * Updated changelog. * 72.2.0 * Updated documentation. * [EuiDataGrid] Fix nested interactive controls axe error (#6517) * Fix nested interactive controls error in column visibility control * Restore mouse experience by making switch "label" draggable - this (almost) restores previous draggable functionality - the column name and handle are both mouse draggable, while the switch is not as it is its own interactive thing * Fix nested interactive controls error in column sorting control - the X button and sort button group should not be draggable as they are interactive * Improve mouse experience by making column name draggable - The x button and sort button group are interactive and cannot be draggable, but the column name/token can be + tweak padding to belong to the column name, for extra grabbable area * [opinionated] Fix multiple CSS styles to actually work correctly - whether or not these style changes are wanted will have to be approved by a designer * changelog * Improve Kibana upgrade docs (#6518) - add table of contents - order steps more generally in chronological order so it's easier to follow vs jumping around the page - update recommendations and general context * [Docs] Update `EuiConfirmModal` examples (#6519) * [Docs] Update `EuiConfirmModal` examples * Adding a width * Update src-docs/src/views/modal/confirm_modal.tsx Co-authored-by: florent-leborgne <[email protected]> Co-authored-by: florent-leborgne <[email protected]> * [EuiToolTip] Enforce only one visible tooltip at a time (#6520) * [misc cleanup] Group relative imports by general concept - keep parent services together, keep tooltip-specific imports together * Add tooltip manager that hides all other tooltips when a new tooltip is shown * Write Cypress E2E tests for multiple tooltip behavior * Changelog * Unit tests for control of internal tooltip visibility state via `ref` (#6522) * [Docs] Fix code snippet (#6526) * Added a11y specs for EuiNotificationEvent, EuiPageHeader, EuiPortal (#6524) * Added a11y specs for EuiNotificationEvent, EuiPageHeader, EuiPortal * Added a11y and keyboard specs for EuiNotificationEvent. * Added page header a11y tests. * Added a11y and keyboard specs for EuiPortal. * Simplifying Portal test, adding breadcrumb and tabs to Page Header. * Simplified two components, added a state check to EuiNotification. * Further reducing the EuiPortal specs to test just that one component. * update i18ntokens * Updated changelog. * 73.0.0 * Updated documentation. * Check that the `upstream` remote is correctly set before proceeding in release script (#6528) * [EuiModalHeaderTitle] Remove automatic detection of title contents in favor of always wrapping a `h1`, configurable via new `component` prop (#6530) * Use a `component` prop for EuiModalHeaderTitle tag wrapper instead of trying to detect child types - this is because Kibana's `<FormattedMessage>` component (which only outputs a string) is incorrectly not getting styles applied to it * Add prop unit tests + convert tests to RTL while we're here * Allow `EuiConfirmModal`s to override title component tag as well if necessary * Changelog * tweak changelog copy a bit more * [Docs EuiTable] Replace all the demos using `data_store` with `faker-js` (#6521) * Replacing `data_store` with `faker-js` * Converted `actions.js` to `tsx` * Converted `auto.js` to `.tsx` * Converted `expanding_rows.js` to `*.tsx` * Converted `footer.js` to `*.tsx` * Converted `mobile.js` to `*.tsx` * Converted `selection.js` to `*.tsx` * Converted `sorting.js` to `*.tsx` * Fix typing/`any` usages - provide correct EuiBasicTable typescript usages/definitions * Clean up divs and Fragments * Simplify & clean up delete selected items code + remove it entirely from `footer` and `expanding_rows` examples - those demos really don't need that behavior at all * Remove unnecessary `renderStatus` abstraction - not being used in multiple places, so no need to DRY it out * Fix incorrectly rendering mobile names + DRY out `renderStatus` - not totally clear to me why those examples have such complex mobile examples + add a `mobileOptions.only` example in the `mobile` demo to make up for removed examples * Remove unused `sortable` - there's no sorting being passed to the table, so it does nothing * Clean up table layout example - remove need for randomly generated group IDs - simplify var names - use value passed directly from `onChange` instead of using a `.find()` - use flex group for button toggles instead of ` `s * Clean up emoji flag logic - move `getEmojiFlag` util to bottom of the file - it's not relevant to anything table-related and shouldn't be the first thing a dev has to read/parse - remove unnecessary extra flag/countryCode() call * Remove unnecessary data length abstraction/logic - if `userLength` isn't being used anywhere else, just hard code it in - no need for a var - remove unnecessary `filteredUsers` logic + remove unnecessary `RIGHT_ALIGNMENT` import * Organize table/component logic by concept - move `columns` out to static init where logical - add headings to sort out manual pagination/sorting logic where it isn't necessarily relevant to the demo (but aids in visual output) - add comment to `findUsers` to more clearly explain what it's doing - remove unnecessary abstractions for longer files * Removed `getEmojiFlag` * Remove lodash `uniqBy` dependency - replace one instance with a `Set` example for uniqueness * Tweak new location column to truncate - this column is now fairly long compared to before - let's restore the previous table visual apperances by truncating * Standardize inconsistent link behavior - it doesn't really make sense to have a last name be a link as opposed to a username - some demos were doing this, some weren't, so standardize the rendering - remove actual external links to github - most of the newly random usernames aren't valid in any case Co-authored-by: Constance Chen <[email protected]> * Removed domain check from EuiLink (#6535) This PR changes the behavior of the EUILink component to apply rel="noreferrer" universally for all external links, as it's considered a best practice. Prior to this PR, there was an exception for elastic.co domains, added in #1565. As this behavior is no longer required, we're opting to remove the exception. * [EuiBasicTable] Fix row heights jumping when actions are disabled (#6538) * [EuiBasicTable] Fix row heights jumping when actions are disabled * Update snapshots * changelog * Adding EuiModal callout for H1 rendering. (#6497) * Adding EuiModal callout for H1 rendering. * Updating text to be more concise and clear. * Updating documentation for default H1 wrapper on EuiModalHeaderTitle. * Removing example callout, added explanation to component type definition. * Removed extra H1 tags from Docs examples. * Removed H1 from EUIModal component and a11y specs. * Update checklist template to recommend `@default` jsdoc usage (#6541) * [Emotion] Convert EuiBasicTable (#6539) * [tech debt] convert `useEuiTheme` tests to RTL `renderHook` - which is generally a nicer API than the one I yolo'd * [tech debt] Add more missing unit tests for `useEuiTheme` * [tech debt] write basic unit test for `withEuiTheme` * Add new `RenderWithEuiTheme` render prop util * Convert `tbody` loading styles to Emotion - I opted not to create a top-level component for this due to the very limited styles being applied, and due to HOC/theme access shenanigans * Fix error/empty states not rendering loading styles - by only rendering one `<tbody>`, not multiple * Write basic `loading` test + switch `render` to RTL * [extra] Massive clean up of EuiBasicTable unit tests - switch to RTL totally (shallow was not handling the new render prop well) - DRY out various repeated props - stop use snapshots for every single test - use specific assertions instead. For visual rendering for various prop combos, we should use Storybook - leave snapshots in for two specific render tests - barebones & kitchen sink props * Delete scss files * Add `shouldRenderCustomStyles` test * changelog * Add affordance for reduced motion media query - this matches how EuiProgress behaves + clean up animation shorthand * Add CSS workaround/fix for visual Safari bug - apparently `position: relative` on the parent and not on the `tbody` was a cross-browser fix :( * [EuiResizablePanel] Added tabindex prop to EuiResizablePanel for keyboard accessibility. (#6534) * Added tabindex prop to EuiResizablePanel for keyboard accessibility. * Added unit test and corrected tabIndex type. * Renaming tabindex to tabIndex for consistency in docs. * Renaming test description. Removing changelog. Co-authored-by: Jason Stoltzfus <[email protected]> Co-authored-by: Elizabet Oliveira <[email protected]> Co-authored-by: florent-leborgne <[email protected]> Co-authored-by: Trevor Pierce <[email protected]> Co-authored-by: Bree Hall <[email protected]>
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.
Summary
This PR replaces all the demos using
data_storewithfaker-jsand closes #5837:*.tsxQA
The best way to QA is to open the demos in CodeSandbox or locally:
General checklist
Follow up
The last example, custom, is a class component. So I'll follow up with a PR with the conversion to a functional component/typescript. The issue can be found here #5959.