fix(Icon): remove extra hidden progress icon from success state - #7788
Conversation
|
Preview: https://pf-pr-7788.surge.sh A11y report: https://pf-pr-7788-a11y.surge.sh |
thatblindgeye
left a comment
thatblindgeye
left a comment
There was a problem hiding this comment.
@mcoker can you just confirm we don't want the __progress container at all for an "in progress" icon that shows a loaded/completed state? Based on docs that's the case but just want to confirm in case we want to keep the container for any future reason.
|
@thatblindgeye good question! It does look like the CSS for in progress icons are setup in a way that they may have transitioned/faded between the progress spinner and whatever icon replaces it at some point, but it doesn't transition currently. patternfly/src/patternfly/components/Icon/icon.scss Lines 204 to 207 in 3d9bacd I'd say matching the react implementation is what we should do for core, then we can follow up with design and see if there is supposed to be a transition with the icons to/from in-progress, and if so address that in a future issue. No need to hold up this PR as long as core matches react. @lboehling @kaylachumley wdyt? For an "in progress" icon that could have some icon in it, then on some action, the icon is replaced with a spinner, then when the action is complete, the spinner goes away and either the original or a new icon is shown - should the icons just change immediately? Or should the they fade in/out, so the old icon fades out and the spinner fades in, and when it's done, the spinner fades out and the new icon fades in? |
|
🎉 This PR is included in version 6.3.0-prerelease.65 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
Closes #7285.