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.

Sign up for GitHub

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

Jump to bottom

Progress page: remove toggle animation button #34787

Merged
merged 2 commits into from
Aug 21, 2021
Merged

Progress page: remove toggle animation button #34787

merged 2 commits into from
Aug 21, 2021

Conversation

XhmikosR
Copy link
Member

@XhmikosR XhmikosR commented Aug 20, 2021 •
edited
Loading

We don't use the same approach with a button on the Placeholders page.

Preview: https://deploy-preview-34787--twbs-bootstrap.netlify.app/docs/5.1/components/progress/#animated-stripes

@XhmikosR XhmikosR requested a review from mdo August 20, 2021 15:23
@XhmikosR
Progress page: remove toggle animation button
540f437 
We don't use the same approach with a button on the Placeholders page.
@XhmikosR XhmikosR force-pushed the main-xmr-docs-rm-prog-button branch from 3a79ca5 to 540f437 Compare August 20, 2021 15:26
@XhmikosR XhmikosR marked this pull request as ready for review August 20, 2021 15:26
@XhmikosR XhmikosR requested a review from a team as a code owner August 20, 2021 15:27
ffoodd
ffoodd approved these changes Aug 20, 2021
View reviewed changes
Copy link
Member

@ffoodd ffoodd left a comment

Choose a reason for hiding this comment

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

LGTM! Was this done on purpose for accessibility, maybe? Ping @patrickhlauke just in case 😄

@patrickhlauke
Copy link
Member

LGTM! Was this done on purpose for accessibility, maybe? Ping @patrickhlauke just in case 😄

nah, i'm pretty sure that was well before my time, and it was there mainly to dynamically demonstrate how adding the class started the animation. no concerns with removing it (as this obeys prefers-reduced-motion which in my book counts as a mechanism to pause/stop/hide animations)

@ffoodd
Copy link
Member

ffoodd commented Aug 20, 2021

:shipit:

@XhmikosR
Copy link
Member Author

The main reason for having the toggle button is that this animations are not GPU accelerated thus they crippled performance on low-end devices.

But since we do not have a button in Placeholders, this makes things consistent . We can bring it back if needed.

@XhmikosR XhmikosR merged commit 9e1d81a into main Aug 21, 2021
@XhmikosR XhmikosR deleted the main-xmr-docs-rm-prog-button branch August 21, 2021 03:54
alexkoala added a commit to arumduri/bootstrap that referenced this pull request Sep 10, 2021
@alexkoala
Translation update from v5.1.1
edd1b07 
twbs/bootstrap#34787
twbs/bootstrap#34877
twbs/bootstrap#34886
twbs/bootstrap#34892
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ffoodd ffoodd ffoodd approved these changes

@mdo mdo Awaiting requested review from mdo

Assignees
No one assigned
Labels
docs v5
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@XhmikosR @patrickhlauke @ffoodd