feat(expressive-loader): new module #2260

cordeliadillon · 2024-02-02T18:12:50Z

Address #2162

This PR contains CSS changes
This PR does not contain CSS changes

Description

Introduce styles for an expressive loader pattern, including standard and reduced motion modes.

Notes

The full experience of the expressive loader pattern requires Javascript to rotate through the loading messages. I've noted in the documentation how Javascript could be used to achieve the desired behavior and have also included basic functionality in main.js. I have a full Marko implementation in the 2016-expressive-loader branch of ebayui-core, if you'd like to see it in action.

There are several potential implementation techniques for line animations, including canvas; I opted for a CSS version since, well, a canvas implementation wouldn't add much to Skin.

Motion approaches

	Standard	Reduced motion
Initial message display	For large text, the first message fades in after a delay. For medium text, the first message appears immediately.	For large text, the first message fades in immediately. For medium text, the first message appears immediately.
Message swapping	Messages animate in and out vertically. Message display time is controlled in JS.	Messages do not animate in and out, but simply change from one string to another. Messages should display for longer than the standard experience; duration determined in JS.
Line animation	Colorful lines are assigned a randomized max width. They start on the left at width 0 and grow horizontally to their max width while moving right across the screen.	Colorful lines have somewhat random widths. They start on the left and move right across the screen. They do not scale and move more slowly than the lines in the standard motion version.

Screen reader experience

The expressive loader consists of two pieces: the message container and the progress bar.

Message container

Expressive loader can have up to 3 messages that loop. As the current message fades out, a new message fades in. The current message has the following accessible properties:

role: status
name: {current message}
aria-live: not specified, but implied value of polite due to status role

As a live region with role status, the current message container will read out its content whenever it changes.

A new message animating in has aria-hidden="true" to hide it from the screen reader. While the new message and the current (outgoing) message are briefly visible on the screen at the same time, the current (outgoing) message is the only one present in the accessibility tree.

Progress bar

role: progressbar
name: "Loading..." || {custom label}
description: {current message}
aria-valuenow: not specified, indicating progress percentage is indeterminate

The progress bar is an indeterminate progressbar. While there's a native <progress> HTML element, its style overrides are limited, so we use a div with role progressbar. The progress bar's accessible name defaults to "Loading..." but can be customized, for example to support localization. The progress bar is aria-describedby the current message, if one exists.

We did explore using aria-roledescription="Loading indicator" instead of an aria-label, but aria-roledescription has limited support across mobile browsers, confirmed in a brief CodePen test.

Screenshots

Standard

Medium text

Long message

Videos

Default (w/ Javascript to handle message changes)

FullAnimation-Default.mov

Reduced motion (w/ Javascript to handle message changes)

FullAnimation-ReducedMotion.mov

Browser limitations: Safari < 14

This CSS-only implementation of expressive loader line animation dynamically updates the order property of the lines to achieve the appearance of infinite lines. Unfortunately, Safari < 14 does not support animating flex order, as I explored in this Codepen: https://codepen.io/cordelia/pen/gOyOePQ

Here is what the default expressive loader looks like on Safari 12:

iOS-Safari-12.0-Default.mov

Here is the same animation on Safari 14:

iOS-Safari-14.0-Default.mov

Checklist

I verify the build is in a non-broken state
I verify all changes are within scope of the linked issue

I regenerated all CSS files under dist folder
I tested the UI in all supported browsers
~~I did a visual regression check of the components impacted by doing a Percy build and approved the build~~
This is a net new component
I tested the UI in dark mode and RTL mode
I added/updated/removed Storybook coverage as appropriate

ArtBlue

I reviewed everything except for the stories. I'm stopping here because there's likely going to be large changes based on my feedback that will likely impact structure. We can tackle the stories after we've ironed everything else out and the structure is solidified.

ArtBlue · 2024-02-08T15:42:26Z

dist/tokens/evo-light.css

@@ -72,6 +72,10 @@
    --color-data-viz-chart-quinary-stroke: var(--color-teal-5);
    --color-data-viz-tooltip-shadow-primary: #00000026;
    --color-data-viz-tooltip-shadow-secondary: #0000002b;
+    --color-expressive-loader-red: #e53238;


I have to say, I'm feeling more and more uneasy about having very module-specific color definitions, especially non-tokenized (one-offs) inside evo. While the typical props serve the vast majority of Skin, these are very specific and limited. Having these load with a "global" file seems a bit out of place. Devs who don't use expressive-loader would be loading these module-specific props without ever needing them.

I think these types of light and dark mode props would be better suited to be defined in the module files instead.

ArtBlue · 2024-02-08T15:44:01Z

docs/_includes/expressive-loader.html

+  <p>
+    The expressive loader can replace our traditional spinner to feel faster during periods of longer loading.
+    To portray responsive and quicker loads, the expressive loader's design uses visual momentum to
+    indicate progression across refreshing, page loading and user actions


Missing period.

Suggested change

indicate progression across refreshing, page loading and user actions

indicate progression across refreshing, page loading and user actions.

ArtBlue · 2024-02-08T15:45:22Z

docs/_includes/expressive-loader.html

+    It consists of an animated progress bar in eBay brand colors and an optional set of messages that are cycled into
+        view. It also supports a reduced motion mode.


Would be best to avoid unnatural line breaks.

Suggested change

It consists of an animated progress bar in eBay brand colors and an optional set of messages that are cycled into

view. It also supports a reduced motion mode.

It consists of an animated progress bar in eBay brand colors and an optional set of messages that are cycled into view. It also supports a reduced motion mode.

docs/_includes/expressive-loader.html

src/less/expressive-loader/expressive-loader.less

ArtBlue · 2024-02-08T16:53:32Z

src/less/expressive-loader/expressive-loader.less

+        }
+    }
+
+    .expressive-line-animation__reduced-motion {


I'm not sure why we need this class since reduced motion should work out of the box in a media query, right?

ArtBlue · 2024-02-08T17:00:19Z

docs/_includes/expressive-loader.html

+    <div class="demo__inner">
+      <div class="expressive-loader">
+        <div role="progressbar" aria-label="Loading..." class="expressive-line-animation">
+          <div class="expressive-line-animation__reduced-motion">


It's probably not ideal to have different markup for reduced motion since it's a client site setting that is queried in css. Is it possible to come up with a singular markup structure that would work in both cases - regular and reduced motion?

Agreed. I think this is the main change that is required right now. Getting it all onto a single, flexible markup structure (even if it might end up containing a little bit of redundancy).

cordeliadillon

@ArtBlue, thank you for all your awesome feedback! I've just pushed a new commit.

High level overview of changes

Removed nesting in less file and simplified BEM naming
Moved color tokens into the expressive-loader file, made color references more brand agnostic
Cleaned up DOM structure a bit (see reduced motion notes below)
Using media queries instead of classes for motion-specific CSS
General doc improvements, including rearranging examples so that reduced motion & non-reduced motion examples are intermingled

Reduced motion

Structure

I saw your comment about standardizing on the same DOM structure for both reduced and non-reduced motion. I've made them closer to each other, but am not sure I can get them exactly the same given the different types of animations. The reduced motion one is essentially a marquee (oh dear, remember <marquee>) so its content is a little more rigid.

Here's what I landed on in the commit -- I welcome any suggested improvements:

No reduced motion

<div role="progressbar" aria-label="Loading..." class="expressive-loader__progress">
  <div class="expressive-loader__lines">
    <!-- dynamically add / remove child lines here -->
  </div>
</div>

Reduced motion

<div role="progressbar" aria-label="Loading..." class="expressive-loader__progress">
  <div class="expressive-loader__lines">
    <!-- Set A lines here -->
  </div>
  <div class="expressive-loader__lines">
    <!-- Set B lines here -->
  </div>
  <div class="expressive-loader__lines">
    <!-- Set A lines here  again-->
  </div>
  <div class="expressive-loader__lines">
    <!-- Set B lines here again -->
  </div>
</div>

Examples

I initially was using a class (rather than media query), so I could clearly and consistently demonstrate the difference between the two motion modes in the docs. Because the markup & behavior is a little different between the two, the examples in the docs and in Storybook might not render well depending on the viewer's system settings. Is that okay?

cordeliadillon · 2024-02-13T19:21:03Z

src/tokens/evo-core.css

@@ -93,6 +93,7 @@
    --font-size-18: 1.125rem;
    --font-size-20: 1.25rem;
    --font-size-24: 1.5rem;
+    --font-size-28: 1.75rem;


👀 Not sure if I should add this here, but our Large Text typography style is size 20 / line height 28.

For typography, this is good. For line height, we use spacing props.

src/less/expressive-loader/expressive-loader.less

cordeliadillon · 2024-02-13T22:28:15Z

docs/_includes/expressive-loader.html

+      Add an event listener for the element's <code>animationend</code>, which triggers the creation of another line and
+      removal of the oldest line.
+    </li>
+    <li>Prepend the line to the <code>.expressive-loader__lines` container.</code></li>


Suggested change

<li>Prepend the line to the <code>.expressive-loader__lines` container.</code></li>

<li>Prepend the line to the <code>.expressive-loader__lines</code> container.</li>

In Skin docs, we typically use <span class="highlight"></span> to format/highlight certain text. Let's keep it consistent and use that instead of <code>.

docs/_includes/expressive-loader.html

ArtBlue

Doing another round of review...

ArtBlue · 2024-02-14T15:56:19Z

docs/_includes/expressive-loader.html

+      Add an event listener for the element's <code>animationend</code>, which triggers the creation of another line and
+      removal of the oldest line.
+    </li>
+    <li>Prepend the line to the <code>.expressive-loader__lines` container.</code></li>


In Skin docs, we typically use <span class="highlight"></span> to format/highlight certain text. Let's keep it consistent and use that instead of <code>.

src/less/expressive-loader/expressive-loader.less

ArtBlue · 2024-02-14T16:02:40Z

src/less/expressive-loader/expressive-loader.less

@@ -26,7 +40,7 @@
    color: var(--color-foreground-primary);
    font-size: var(--font-size-large-1);
    font-weight: var(--font-weight-regular);
-    line-height: 28px;
+    line-height: var(--font-size-28, 28px);


For line-height, we use the spacing props in evo-core. Oh, and, you don't need a backup default.

Our font vars use rem units, our spacing vars use px units. A concern I have about using spacing vars for line height is that if someone were to update their base font size to something larger to make the page mor readable, the line height wouldn't scale appropriately.

ArtBlue · 2024-02-14T16:03:13Z

src/less/expressive-loader/expressive-loader.less

@@ -36,141 +50,127 @@
 .expressive-loader--medium .expressive-loader__message-container {
    font-size: var(--font-size-medium);
    font-weight: var(--font-weight-bold);
-    line-height: 24px;
+    line-height: var(--font-size-24);


...spacing props in evo-core.

ArtBlue · 2024-02-14T16:42:59Z

src/tokens/evo-core.css

@@ -93,6 +93,7 @@
    --font-size-18: 1.125rem;
    --font-size-20: 1.25rem;
    --font-size-24: 1.5rem;
+    --font-size-28: 1.75rem;


For typography, this is good. For line height, we use spacing props.

src/less/expressive-loader/expressive-loader.less

docs/_includes/expressive-loader.html

changeset-bot · 2024-02-22T20:13:59Z

🦋 Changeset detected

Latest commit: aea7e9d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@ebay/skin	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

cordeliadillon · 2024-02-22T20:18:03Z

@ArtBlue @ianmcburnie, major updates since last review:

Rearchitect to have same HTML structure for default and reduced motion
Using only CSS animations for line animation (thanks @LuLaValva for pointing in the right direction here with flex order. Genius!)
Streamlined documentation

…or animation

ArtBlue

Looks great overall! Just some minor stuff...

ArtBlue · 2024-03-05T14:51:57Z

src/less/expressive-loader/expressive-loader.less

+                    (
+                        (
+                                var(--expressive-loader-line-scale-duration) /
+                                    var(--expressive-loader-line-count)
+                            ) * 2
+                    ))
+    );


Did prettier insist on the indentations in these blocks? If not, it would be great to do 4 space indentations from previous lines.

Yes, Prettier keeps forcing this indentation!

src/less/expressive-loader/expressive-loader.less

ArtBlue · 2024-03-05T15:04:39Z

docs/_includes/expressive-loader.html

+      <div class="expressive-loader">
+        <div class="expressive-loader__message-container">
+          <div aria-hidden="true" class="expressive-loader__message">Hang tight.</div>
+          <div role="status" id="expressive-loader-message-1" class="expressive-loader__message expressive-loader__message--initial"></div>


Having this 'with Message' section here without a message present seems odd. I know you describe the reason above, but the message should be there statically anyway. We can revise the messaging as we do in other places to indicate the messaging interaction piece is not included in Skin and needs to be added using a library or js.

ArtBlue · 2024-03-05T15:08:06Z

docs/_includes/expressive-loader.html

+  {% endhighlight %}
+
+  <h4>
+    Multiple messages (default behavior)


Generally, for interactive modules, we actually build a dumbed-down interaction portion as well in Skin using main.js. For this section, it might be a good idea to do so. You can include a blurb about the interaction piece requiring js similar to how we've done for other modules.

ArtBlue · 2024-03-05T15:20:31Z

docs/_includes/expressive-loader.html

+  </p>
+
+  <h4>
+    Multiple messages (reduced motion behavior)


We typically go from simplest implementation to most complex, so single message should come before multiple messages. In fact, I think top-level sections are probably sufficient:

h3 - Default Expressive Loader h3 - Expressive Loader with One Message h3 - Expressive Loader with Multiple Messages (in this section include a paragraph about reduced motion)

I don't think it's necessary to highlight reduced motion so much, especially in section titles. It's more of a "transparent" feature for users that devs don't necessarily need to know too much details about.

docs/_includes/expressive-loader.html

src/less/expressive-loader/stories/expressive-loader.stories.js

ArtBlue · 2024-03-05T15:37:33Z

src/less/expressive-loader/stories/expressive-loader.stories.js

+</div>
+`;
+
+export const withMessagesInInitialState = () => `


I'm not sure we need initial state coverage.

…lean up examples

cordeliadillon · 2024-03-05T19:01:39Z

docs/src/js/main.js

@@ -193,6 +193,55 @@ document.querySelectorAll('.dialog-button').forEach(function (el) {
    dialogWidget._el.addEventListener('dialog-cta', logEvent);
 });

+// EXPRESSIVE-LOADER
+// Simple expressive loader implementation that handles rotating messages.
+class ExpressiveLoader {


@ArtBlue Here's a simple JS implementation of the message rotation. There's more nuance I'd bring to a robust implementation (in ebayui-core and/or makeup-js) around timing, reduced motion enhancements, and start states, but opted to just do message rotation for the Skin examples.

ArtBlue

Look great to me! One more small tweak.

Let's see if Andrew and/or Ian have feedback.

ArtBlue · 2024-03-05T19:35:42Z

docs/_includes/expressive-loader.html

+    Expressive Loader with Multiple Messages
+  </h3>
+  <p>
+    For the following examples, we're using Javascript to rotate through messages.


We should be explicit that this js is not included in Skin. We do this throughout Skin docs. here's one instance:

LuLaValva

Awesome stuff! I think this should be good to merge.

ArtBlue

Looks good. One last tweak: let's just avoid people possibly assuming that when we say "We're using Javascript..." that the interaction is built into Skin. Being super clear that it's not avoids that risk altogether.

ArtBlue

Looks great! Thanks, @cordeliadillon ! I love where we landed with this.

cordeliadillon changed the title ~~2162 expressive loader~~ feat(expressive-loader): new module Feb 2, 2024

cordeliadillon requested review from ArtBlue and ianmcburnie February 2, 2024 18:30

cordeliadillon marked this pull request as ready for review February 2, 2024 19:20

cordeliadillon requested a review from agliga February 2, 2024 22:44

ArtBlue requested changes Feb 8, 2024

View reviewed changes

cordeliadillon commented Feb 13, 2024

View reviewed changes

cordeliadillon requested a review from ArtBlue February 13, 2024 20:31

cordeliadillon commented Feb 13, 2024

View reviewed changes

ianmcburnie reviewed Feb 13, 2024

View reviewed changes

docs/_includes/expressive-loader.html Show resolved Hide resolved

ArtBlue reviewed Feb 14, 2024

View reviewed changes

agliga force-pushed the 17.2.0 branch from e7ae739 to c21ea79 Compare February 22, 2024 17:49

cordeliadillon force-pushed the 2162-expressive-loader branch from b721261 to 5b4e8ad Compare February 22, 2024 20:13

cordeliadillon requested a review from ArtBlue February 22, 2024 20:15

cordeliadillon requested a review from ianmcburnie February 26, 2024 18:42

github-actions bot mentioned this pull request Feb 27, 2024

ci: release #2279

Closed

Base automatically changed from 17.2.0 to master February 28, 2024 16:12

cordeliadillon added 11 commits February 29, 2024 09:54

feat(expressive-loader): new module

08de40d

fix(expressive-loader): add timing variables

721091b

docs(expressive-loader): add DS version

b4c4287

fix(expressive-loader): use aria attributes in selectors

771b52a

fix(expressive-loader): animation variables

1a48119

fix(expressive-loader): update spacing

8954271

docs(expressive-loader): reformat storybook

b96f3cd

refactor: incorporate code review feedback

a6f2bbb

refactor: respond to code review comments and use css only approach f…

1ea37a8

…or animation

fix: run build

135fe32

fix(expressive-loader): use CSS grid instead of flex for container

a4820b8

refactor(expressive-loader): simplified css only approach

979cf7e

cordeliadillon force-pushed the 2162-expressive-loader branch from 5b4e8ad to 979cf7e Compare March 4, 2024 20:24

cordeliadillon changed the base branch from master to 17.3.0 March 4, 2024 20:52

ArtBlue requested changes Mar 5, 2024

View reviewed changes

fix(expressive-loader): clean up CSS, add simple JS implementation, c…

3485e2a

…lean up examples

cordeliadillon requested a review from ArtBlue March 5, 2024 18:58

cordeliadillon commented Mar 5, 2024

View reviewed changes

ArtBlue reviewed Mar 5, 2024

View reviewed changes

docs(expressive-loader): update javascript note in docs

ba62d79

LuLaValva approved these changes Mar 12, 2024

View reviewed changes

cordeliadillon requested a review from ArtBlue March 12, 2024 19:00

ArtBlue requested changes Mar 13, 2024

View reviewed changes

ArtBlue approved these changes Mar 13, 2024

View reviewed changes

ArtBlue assigned cordeliadillon Mar 13, 2024

chore(expressive-loader): create changeset

aea7e9d

ArtBlue merged commit 7607f6d into 17.3.0 Mar 13, 2024
1 check passed

ArtBlue deleted the 2162-expressive-loader branch March 13, 2024 17:11

github-actions bot mentioned this pull request Mar 13, 2024

ci: release (next) #2285

Merged

github-actions bot mentioned this pull request Mar 21, 2024

ci: release #2301

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(expressive-loader): new module #2260

feat(expressive-loader): new module #2260

cordeliadillon commented Feb 2, 2024 •

edited

Loading

ArtBlue left a comment

ArtBlue Feb 8, 2024

ArtBlue Feb 8, 2024

ArtBlue Feb 8, 2024

ArtBlue Feb 8, 2024

ArtBlue Feb 8, 2024

ianmcburnie Feb 14, 2024

cordeliadillon left a comment

cordeliadillon Feb 13, 2024

ArtBlue Feb 14, 2024

cordeliadillon Feb 13, 2024

ArtBlue Feb 14, 2024

ArtBlue left a comment

ArtBlue Feb 14, 2024

ArtBlue Feb 14, 2024

cordeliadillon Feb 22, 2024

ArtBlue Feb 14, 2024

ArtBlue Feb 14, 2024

changeset-bot bot commented Feb 22, 2024 •

edited

Loading

cordeliadillon commented Feb 22, 2024

ArtBlue left a comment

ArtBlue Mar 5, 2024

cordeliadillon Mar 5, 2024

ArtBlue Mar 5, 2024

ArtBlue Mar 5, 2024

ArtBlue Mar 5, 2024

ArtBlue Mar 5, 2024

cordeliadillon Mar 5, 2024

ArtBlue left a comment

ArtBlue Mar 5, 2024

LuLaValva left a comment

ArtBlue left a comment

ArtBlue left a comment

	indicate progression across refreshing, page loading and user actions
	indicate progression across refreshing, page loading and user actions.

		It consists of an animated progress bar in eBay brand colors and an optional set of messages that are cycled into
		view. It also supports a reduced motion mode.

	<li>Prepend the line to the <code>.expressive-loader__lines` container.</code></li>
	<li>Prepend the line to the <code>.expressive-loader__lines</code> container.</li>

feat(expressive-loader): new module #2260

feat(expressive-loader): new module #2260

Conversation

cordeliadillon commented Feb 2, 2024 • edited Loading

Description

Notes

Motion approaches

Screen reader experience

Message container

Progress bar

Screenshots

Standard

Medium text

Long message

Videos

Default (w/ Javascript to handle message changes)

Reduced motion (w/ Javascript to handle message changes)

Browser limitations: Safari < 14

Checklist

ArtBlue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cordeliadillon left a comment

Choose a reason for hiding this comment

High level overview of changes

Reduced motion

Structure

No reduced motion

Reduced motion

Examples

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ArtBlue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

changeset-bot bot commented Feb 22, 2024 • edited Loading

🦋 Changeset detected

cordeliadillon commented Feb 22, 2024

ArtBlue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ArtBlue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LuLaValva left a comment

Choose a reason for hiding this comment

ArtBlue left a comment

Choose a reason for hiding this comment

ArtBlue left a comment

Choose a reason for hiding this comment

cordeliadillon commented Feb 2, 2024 •

edited

Loading

changeset-bot bot commented Feb 22, 2024 •

edited

Loading