Skip to content

[EuiSkeleton] Design review#6560

Merged
elizabetdev merged 1 commit intoelastic:feature/skeletonsfrom
elizabetdev:design-review
Jan 27, 2023
Merged

[EuiSkeleton] Design review#6560
elizabetdev merged 1 commit intoelastic:feature/skeletonsfrom
elizabetdev:design-review

Conversation

@elizabetdev
Copy link
Copy Markdown
Contributor

Summary

This PR makes the EuiSkeletonText and EuiSkeletonTitle have a more rectangular border-radius.

Design changes

skeleton@2x

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

  • Checked in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox

@elizabetdev elizabetdev added the skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) label Jan 27, 2023
@elizabetdev elizabetdev requested a review from cee-chen January 27, 2023 19:44
<EuiText>
<p>
<strong>EuiSkeletonCircle</strong> allows you define a large and
<strong>EuiSkeletonRectangle</strong> allows you define a large and
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha whoops, nice catch!

Copy link
Copy Markdown
Contributor

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me!

@kibanamachine
Copy link
Copy Markdown

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

@elizabetdev elizabetdev merged commit f9161a1 into elastic:feature/skeletons Jan 27, 2023
cee-chen added a commit that referenced this pull request Jan 31, 2023
* EuiSkeleton components (#6502)

* 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 <elizabet.oliveira@elastic.co>

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

Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>

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

Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>

* 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 <elizabet.oliveira@elastic.co>
Co-authored-by: Constance Chen <constance.chen.3@gmail.com>

* Design review (#6560)

* Deprecate `EuiLoadingContent` in favor of `EuiSkeletonText` (#6557)

* 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

* Fix Emotion styles var name

* [EuiSkeletonLoading] Improve loading accessibility and bake in `isLoading` 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

* [PR feedback] Add `children` to playground

- Missed in #6562

- Playground still isn't perfect; but good enough for now

* Tweak default `EuiSkeletonCircle` size to match default `EuiAvatar` size

---------

Co-authored-by: Andrea Della Valle <andreareva@gmail.com>
Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants