feat(ProgressIndicator): add animation #9340

kristastarr · 2021-07-27T23:15:19Z

Progress Indicator Animations:

The blue line at the top of a completed step animates from left to right.
The "current" and "completed" status icons transition in opacity when they appear and disappear.
There is a subtle transition on the optional label text on initial page load.

Changelog

New
Added keyframes and transitions to _progress-indicator.scss

Changed
Changed the ProgressIndicator component in the React package - this change would require a StepData object to be passed in as a prop to the ProgressIndicator component, in order ProgressIndicator to render a ProgressStep. (Currently the ProgressSteps are being rendered as React.Children in ProgressIndicator).

Rationale: Properties of this.props.children cannot be modified, so currently are being re-rendered using React.cloneElement with the updated "current" or "completed" property each time the CurrentIndex changed. When I added the CSS for the animations, this caused all the icons to animate every single time there is a change to the status of any single ProgressStep. Rendering ProgressStep components from inside the ProgressIndicator enabled me to modify the "current" and "completed" properties of each ProgressStep to trigger a re-render of a step/icon only when there is a change to that specific ProgressStep.

Testing / Reviewing

You can see the changes by running the Carbon React Storybook.

recheck

netlify · 2021-07-27T23:15:24Z

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: 483982a

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/616f23ad155fd60008b1729a

😎 Browse the preview: https://deploy-preview-9340--carbon-react-next.netlify.app

github-actions · 2021-07-27T23:15:35Z

DCO Assistant Lite bot All contributors have signed the DCO.

kristastarr · 2021-07-27T23:24:40Z

I have read the DCO document and I hereby sign the DCO.

netlify · 2021-07-27T23:27:00Z

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 483982a

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/616f23ad4c35f10007e3f330

😎 Browse the preview: https://deploy-preview-9340--carbon-elements.netlify.app

netlify · 2021-07-27T23:37:13Z

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: 483982a

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/616f23add63e4800084102bf

😎 Browse the preview: https://deploy-preview-9340--carbon-components-react.netlify.app

johnbister · 2021-08-09T17:06:46Z

The motion looks great to me @kristastarr ! Do you think we should include what errors and disabled states would look like in this pattern as well? For instance, if the user cannot click ahead a few steps because prior steps determine what is displayed in subsequent steps. There was a great discussion with @lisajkaiser about this last week and I think it would be great to include these scenarios as well.

kristastarr · 2021-08-09T21:39:41Z

@johnbister Yes, that makes sense! I'll work on adding the error and disabled states this week.

…invalid icons and helper text

joshblack

The animation looks slick! I think the only big note from the implementation is that it seems like the PR makes a change to ProgressIndicator where it reverts it to a class component versus the function component that we refactored it to recently.

From a motion perspective, will we need to add in support for disabling this animation when the user has a no motion preference? 👀

kristastarr · 2021-09-07T17:19:02Z

@joshblack Absolutely, I added media queries to disable the animations for users who prefer reduced motion.

…ogress-tracker-motion

kristastarr · 2021-09-28T16:09:15Z

@joshblack I made the following updates:

Resolved prior merge conflicts
Updated css to use Carbon motion duration tokens
Removed unnecessary props from ProgressStep that were causing console warnings

One thing to mention (which maybe I should have indicated elsewhere?) is that this will be a breaking change - the format of the props passed to ProgressIndicator has changed

jnm2377 · 2021-10-13T18:44:10Z

packages/react/src/components/ProgressIndicator/ProgressIndicator.js

+export class ProgressStep extends React.PureComponent {
+  constructor(props) {
+    super(props);
+  }


Still trying to figure out what the status of this PR is, but we're actually working on making our components functional components for v11. Devs agreed that it makes more sense esp since most devs these days work with the latest React. We don't want this to be a class component.

Hey, sorry, it looked like I needed to re-write some of the tests to get them to pass and unfortunately I haven't done it yet. I understand what you're saying about functional components, but I could not find a way to apply the animations without making it into a class component. Do you want to close this PR for the moment? I still plan to fix the failing tests, and I will review and seek advice on the feasibility of accomplishing the animation on a functional component.

I'm curious what issues you were running into with the functional component, and also if there are other ways we might be able to implement this without breaking changes. Referring to the stepData prop. Why is that needed instead of handling the props through ProgressStep? It's not our typical API pattern for data of parent/child components like Tabs, Accordion, ContentSwitcher. I think there's a bigger discussion to be had about the implementation.

Please bear with my long explanation here 😅 I'll try to explain how/why I ended up doing it the way I did, and maybe we can come to a better solution - any thoughts or suggestions are greatly appreciated! I definitely agree it would be preferable to not introduce any breaking changes.

The issue I was having was how to prevent a re-render of all the ProgressStep components each time there was a change to the ProgressIndicator's props. Once I added in the CSS keyframes animations, the animations were re-triggered every time each ProgressStep was re-rendered, which caused a weird flashing effect (see below although please note that that is an earlier version that did not yet include the line animations, only the icon animations). The goal was for each ProgressStep & its icon to re-render only when there was a change to its status.

progress-tracker2.mp4

In the current implementation of ProgressIndicator.js, renderSteps() clones each child element each time there is a change to the ProgressIndicator's currentIndex, and so the cloneElement() is what was triggering the re-render. Instead of cloning the element, I wanted the element to persist but just change it's props if necessary. I first thought to use React.memo() but from my understanding, that is not something that can be used with React.Children. So that's how I ended up passing the StepData in to render the ProgressSteps with the newly added isCurrent and isComplete props, rather than rendering them as cloned React.Children. The solution I found was to use React.PureComponent to make use of its comparison of those props to only re-render a ProgressStep if it changes, hence the Class component.

As a next step, I addressed the lint and format errors, and I will take a look at the typical pattern for for handling parent/child components in the components you mentioned. (The PublicAPI-test is still failing but since we may eliminate the changes causing that fail, I will address it later if need be.) @matthewgallo will take a look at this as well and we will hopefully find a solution that will work as a functional component and also one without any breaking changes.

Hey @kristastarr sorry for the late reply. Thank you for that detailed explanation, it's very helpful. I'd really like to help get this into production since it's something our designers are interested in. I think going forward, we can do a few things:

Make this an RFC or create a separate RFC to get more input from the rest of our dev team on the implementation.

I am also planning on bringing this up in our next dev team meeting (this week), and hopefully get some thoughts on how we feel about the API changes. Maybe even think about alternate solutions.

Based on feedback and input from the rest of the team, we can decide what to do, how to help, etc.

How does that sound?

…arr/carbon into progress-tracker-motion

jnm2377 · 2021-12-03T17:38:03Z

Hey @kristastarr After talking to our dev team, we would prefer for the component to keep the existing API, where the steps are rendered as children and not a prop data array. We've just found that our components that follow the data prop pattern can be a pain both for users and maintainers. It can just limit people sometimes. So our requests would be to keep the functional component and API as is.

It seems that this PR hasn't had some activity for quite some time, so I'm going to close this for now. Please feel free to open another PR with these changes once you're ready. 🙂

kristastarr · 2022-01-05T21:22:48Z

I am still planning to work on implementing the animation with a functional component. If yes, I will check in with y’all to see if I should do a new PR to Carbon, and if not we will plan to incorporate it into the Carbon for IBM products.

kristastarr added 3 commits June 30, 2021 14:12

add animation to progress line on each individual progress step

619218d

add animation to progress indicator

d5f1236

remove top border on ProgressStep component in vertical variant

6f11616

kristastarr requested a review from a team as a code owner July 27, 2021 23:15

kristastarr requested review from joshblack and tw15egan July 27, 2021 23:15

kristastarr marked this pull request as draft July 27, 2021 23:18

johnbister self-requested a review July 28, 2021 20:49

johnbister added component: progress-indicator type: enhancement 💡 labels Jul 28, 2021

johnbister mentioned this pull request Aug 10, 2021

Sprint 16: Yentl Yak 🐂 #9356

Closed

55 tasks

kristastarr added 2 commits August 11, 2021 09:55

feat(react): Progress Tracker: add transitions to current, complete, …

9ab421b

…invalid icons and helper text

feat(react): Progress Tracker: add transitions to current, complete, …

3ccbc33

…invalid icons and helper text

kristastarr changed the title ~~Progress tracker motion~~ feat(ProgressIndicator): add animation Aug 11, 2021

kristastarr marked this pull request as ready for review August 26, 2021 17:30

joshblack suggested changes Aug 30, 2021

View reviewed changes

feat(progress-indicator): add media query for prefers-reduced-motion

2f2661e

kristastarr and others added 5 commits September 15, 2021 11:39

experiment with animations for form inputs

b879a0d

Merge branch 'carbon-design-system:main' into input-validation

4c5cea0

add files for cf foundry deploy

c26f694

try to deploy

1d5a790

update maniest.yml

a38a4b3

kristastarr added 2 commits September 28, 2021 08:51

feat(progress-indicator): fix conflicts; use motion duration tokens

7a4af74

Merge branch 'main' of github.com:carbon-design-system/carbon into pr…

c22623b

…ogress-tracker-motion

kristastarr added 8 commits September 30, 2021 08:32

add 5 variations of text input animations

4ef814c

Merge branch 'input-validation'

f9272aa

merge upstream

6feccaf

Merge branch 'main' into progress-tracker-motion

46f4875

merge stuff

87034a3

merge more stuff

45ac268

Merge branch 'main' of github.com:carbon-design-system/carbon

11d83f9

Merge branch 'main' of github.com:carbon-design-system/carbon

e1e58ba

jnm2377 suggested changes Oct 13, 2021

View reviewed changes

kristastarr added 6 commits October 15, 2021 10:58

Merge branch 'main' of github.com:carbon-design-system/carbon

e65ce9a

Merge branch 'main' into progress-tracker-motion

add6fbe

remove unintended content from unrelated explorations

949a429

feat(ProgressIndicator): fix lint and format errors

8962239

Merge branch 'main' of github.com:carbon-design-system/carbon

6d349f4

Merge branch 'main' into progress-tracker-motion

d606734

kristastarr marked this pull request as draft October 19, 2021 17:51

kristastarr and others added 5 commits October 19, 2021 12:52

revert(TextInput): remove unintended changes

8232648

Merge branch 'main' into progress-tracker-motion

604b088

Merge branch 'main' of github.com:carbon-design-system/carbon

a272d2b

Merge branch 'main' into progress-tracker-motion

07d8302

Merge branch 'progress-tracker-motion' of https://github.com/kristast…

483982a

…arr/carbon into progress-tracker-motion

jnm2377 closed this Dec 3, 2021

laurenmrice mentioned this pull request Jan 5, 2022

Progress Indicator Animations #9156

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ProgressIndicator): add animation #9340

feat(ProgressIndicator): add animation #9340

kristastarr commented Jul 27, 2021 •

edited

Loading

netlify bot commented Jul 27, 2021 •

edited

Loading

github-actions bot commented Jul 27, 2021 •

edited

Loading

kristastarr commented Jul 27, 2021

netlify bot commented Jul 27, 2021 •

edited

Loading

netlify bot commented Jul 27, 2021 •

edited

Loading

johnbister commented Aug 9, 2021 •

edited

Loading

kristastarr commented Aug 9, 2021

joshblack left a comment

kristastarr commented Sep 7, 2021

kristastarr commented Sep 28, 2021

jnm2377 Oct 13, 2021

jnm2377 Oct 13, 2021

kristastarr Oct 13, 2021

jnm2377 Oct 13, 2021

kristastarr Oct 19, 2021

jnm2377 Nov 9, 2021

jnm2377 commented Dec 3, 2021

kristastarr commented Jan 5, 2022

feat(ProgressIndicator): add animation #9340

feat(ProgressIndicator): add animation #9340

Conversation

kristastarr commented Jul 27, 2021 • edited Loading

Changelog

Testing / Reviewing

netlify bot commented Jul 27, 2021 • edited Loading

github-actions bot commented Jul 27, 2021 • edited Loading

kristastarr commented Jul 27, 2021

netlify bot commented Jul 27, 2021 • edited Loading

netlify bot commented Jul 27, 2021 • edited Loading

johnbister commented Aug 9, 2021 • edited Loading

kristastarr commented Aug 9, 2021

joshblack left a comment

Choose a reason for hiding this comment

kristastarr commented Sep 7, 2021

kristastarr commented Sep 28, 2021

jnm2377 Oct 13, 2021

Choose a reason for hiding this comment

jnm2377 Oct 13, 2021

Choose a reason for hiding this comment

kristastarr Oct 13, 2021

Choose a reason for hiding this comment

jnm2377 Oct 13, 2021

Choose a reason for hiding this comment

kristastarr Oct 19, 2021

Choose a reason for hiding this comment

jnm2377 Nov 9, 2021

Choose a reason for hiding this comment

jnm2377 commented Dec 3, 2021

kristastarr commented Jan 5, 2022

kristastarr commented Jul 27, 2021 •

edited

Loading

netlify bot commented Jul 27, 2021 •

edited

Loading

github-actions bot commented Jul 27, 2021 •

edited

Loading

netlify bot commented Jul 27, 2021 •

edited

Loading

netlify bot commented Jul 27, 2021 •

edited

Loading

johnbister commented Aug 9, 2021 •

edited

Loading