Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new EuiSkeleton components; deprecate EuiLoadingContent #6558

Merged
merged 7 commits into from
Jan 31, 2023

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Jan 27, 2023

Summary

This PR merges a feature branch of already-reviewed PRs. Please see individual PRs for more details:

QA

General checklist

  • Checked in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs (using @default if default values are missing) and playground toggles
  • Added documentation
    - [ ] Checked Code Sandbox works for any docs examples
  • Added or updated jest and cypress tests
  • Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
  • Updated the Figma library counterpart
    - [ ] A changelog entry exists and is marked appropriately N/A - changelogs were added in individual PRs

* Setup EuiSkeleton files and folders structure

* Moved Skeleton under the Display side menu category

* Component variants setup

* feat: skeleton setup [text, rect, circle]

* feat: added skeleton item draft and other minors

* chore: renamed sizes

* feat: added Heading in favor of Rect component

* Update src-docs/src/views/skeleton/skeleton_example.js

Co-authored-by: Elizabet Oliveira <[email protected]>

* Update src-docs/src/views/skeleton/skeleton_example.js

Co-authored-by: Elizabet Oliveira <[email protected]>

* Update src-docs/src/views/skeleton/skeleton_example.js

Co-authored-by: Elizabet Oliveira <[email protected]>

* chore: wrapped p with EuiText and indentation

* chore: removed useless styles

* chore: centralized and renamed skeleton animation keyfram

* refactor: renamed euiSkeletonHeading to euiSkeletonTitle and updated sizes with euiTitle ones

* chore: renamed EuiSkeletonItem to EuiSkeletonRectangle

* Remove hard-coded `euiSkeletonTitleStyles` heights map

- in favor of calling `euiTitle`'s font utils and grabbing `lineHeight` directly from it

* [docs] Add EuiSwitch toggle to EuiSkeletonTitle tests

- this toggle help demonstrates to consumers how to conditionally switch from loading to non-loading components, and also allows us to test heights

* Add `size` prop to `EuiSkeletonText`

+ add calculations & offsets to account for text scale

+ update docs to allow changing size & toggle between text to test visual behavior

* [docs] Add loading toggle for EuiSkeletonCircle

+ prettier fixes

* [PR feedback] Fix incorrect BEM naming; remove extra modifier classes

- BEM: `__` should only be used for children/descendants, and these are parent-level styles and should use regular camelCasing

- modifier classes removal: we're moving away from setting these extra CSS classes, which do not have any actual CSS tied to them (since we're using Emotion CSS-in-JS instead). The top-level className remains as an API for consumers to hook into for overrides if needed.

* [PR feedback] DRY out all `aria`/a11y attributes to reusable helper

- not all components were correctly receiving all the aria attrs they should have been - this helper fixes that

* [PR feedback] Rename` _skeleton` to `utils`, DRY out repeated gradient CSS

- `utils` better matches naming/architecture convention in other components (+ ignore i18n complaint)

+ tweak gradient size and animation for circle and rectangle components to look a bit better

+ rename global animation to a more generic name/usage

* [PR feedback] DRY out repeated CSS between modifiers
- should exist in the base styles instead

+ bonus - DRY out `logicalSizeCSS` to only require 1 input if both sides are the same

* [PR feedback] Avoid dynamic wildcard Emotion classes - use `style` instead

The reason for this is performance - Emotion generates a completely new hash/className for every single possible permutation of width/height passed to it. We want to avoid this for wildcard prop-based styles and use inline styles instead

* [PR feedback] Improve/add unit tests

+ cover all props

+ prefer RTL over Enzyme (part of ongoing migration)

+ fix type issue found during testing

* [docs] Add toggle to EuiSkeletonRectangle example

* [PR feedback] EuiSkeletonRectangle tweaks

- allow numbers as well as strings - now that we're using inline `style`s for the dimensions, React accepts numbers as well (which matches EuiImage API)

- remove image URI in favor of a picsum link

- set width/height on img to prevent page jumping

* [PR feedback] Tweak EuiSkeletonText typing

- for whatever reason, using `|` prints the line numbers in non-ascending order in the props table - using an array and `as const` fixes this, and is reusable in tests

* [PR feedback] Convert docs files to Typescript

+ improve mobile display of demos

* [PR feedback] Documentation copy + order

- move EuiSkeletonCircle slightly lower in docs order - EuiSkeletonText is likely going to be more popular since it already exists

- misc copy tweaks

* [Docs] Set up playgrounds

* DRY out aria-label further

- since `euiLoadingStrings.ariaLabel` already exists, just reuse that instead of making our translators re-translate the same string

* Add fix for Safari workaround

- note: this is already broken on prod as well for `EuiLoadingContent`

* Changelog

* chore: comment typo and zero value for border radius

---------

Co-authored-by: Elizabet Oliveira <[email protected]>
Co-authored-by: Constance Chen <[email protected]>
@cee-chen cee-chen changed the title [Feature] Add new EuiSkeleton components Add new EuiSkeleton components; deprecate EuiLoadingContent Jan 27, 2023
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6558/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6558/

* Deprecate EuiLoadingContent + dogfood EuiSkeletonText

* Remove unnecessary tests

- leave a basic `is rendered` test in just to confirm the content, but should be deleted in the future

* Update documentation with deprecation notice

* changelog
@cee-chen cee-chen marked this pull request as ready for review January 30, 2023 17:17
@cee-chen
Copy link
Contributor Author

Once staging is done updating, I'm going to take another QA pass against the checklist and if everything checks out, merge this in. CC @miukimiu in case you want to take another pass as well

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6558/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6558/

cee-chen and others added 2 commits January 31, 2023 11:21
…ding` API handling (#6562)

* Create reusable `EuiSkeletonLoading` component

- for DRYing out correct accessibility attributes, wrappers, and a simpler loading API

* Convert `EuiSkeletonText` to dogfood new `EuiSkeletonLoading` wrapper

+ update docs/example to how `isLoading`/`children` behavior

* Convert `EuiSkeletonTitle` to dogfood new `EuiSkeletonLoading` wrapper

+ update docs/example to how `isLoading`/`children` behavior

* Convert `EuiSkeletonRectangle` to dogfood new `EuiSkeletonLoading` wrapper

+ update docs/example to how `isLoading`/`children` behavior

* Convert `EuiSkeletonRectangle` to dogfood new `EuiSkeletonLoading` wrapper

+ update docs/example to how `isLoading`/`children` behavior

* Add docs example + a11y callout for `EuiSkeletonLoading`

* Fix annoying Safari bug popping up again

- the new `aria-busy` wrapper appears to make the `z-index` workaround not work, so use `isolation` (https://developer.mozilla.org/en-US/docs/Web/CSS/isolation) instead to force the necessary stacking context

* changelog

* [PR feedback] use `div` instead of `section`

(or in this case, `EuiPanel` for looks
- Missed in #6562

- Playground still isn't perfect; but good enough for now
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6558/

@cee-chen
Copy link
Contributor Author

cee-chen commented Jan 31, 2023

@miukimiu I'm not sure how the community Figma link works, but it looks like the new skeleton components aren't synced to it yet - any chance you could check it and make sure it's all good to go?

I'm going to merge this PR in without it, but would super appreciate making sure we get that last checkbox taken care of post-merge!

@cee-chen cee-chen enabled auto-merge (squash) January 31, 2023 20:25
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6558/

@cee-chen cee-chen disabled auto-merge January 31, 2023 22:22
@cee-chen cee-chen merged commit a8de8df into main Jan 31, 2023
@cee-chen cee-chen deleted the feature/skeletons branch January 31, 2023 22:23
@cee-chen
Copy link
Contributor Author

@AndreaDellaValle woohoo - this is shipped and should be landing in our next EUI release tomorrow! Thank you again for the fantastic community contribution, it's incredibly appreciated!

@AndreaDellaValle
Copy link
Contributor

Amazing @cee-chen !
Following the upgrades, I saw the great work with EuiSkeletonLoading also! 😎

I'm super happy to see this merged into EUI 💜
Thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants