Skip to content

Transitions animations #38296

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.

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ChellyAhmed
Copy link
Contributor

@ChellyAhmed ChellyAhmed commented Mar 21, 2023

Description

This pull request adds modifier classes in helpers/animations.scss (new file). These classes are as follows:

.animation-direction-normal{
  animation-direction: normal;
}
.animation-direction-reverse{
  animation-direction: reverse;
}
.animation-direction-alternate{
  animation-direction: alternate;
}
.animation-direction-alternate-reverse{
  animation-direction: alternate-reverse;
}

These classes are there to alter the animation behavior (direction) of any component, such as spinners.

Motivation & Context

This PR comes as a first step to solving #38150 . Another PR (#38167) introduced an alternative solution by @oraliahdz .

Type of changes

  • [] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

https://deploy-preview-38296--twbs-bootstrap.netlify.app/docs/5.3/helpers/animations/

Related issues

Fixes #38150

@ChellyAhmed ChellyAhmed marked this pull request as ready for review March 21, 2023 20:30
@ChellyAhmed ChellyAhmed requested a review from a team as a code owner March 21, 2023 20:30
@XhmikosR XhmikosR force-pushed the transitions-animations branch from f186982 to 6e5ea94 Compare March 22, 2023 07:28
Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

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

Feel like these should be generated with the utility API and as helpers as they are single property-value pairs.

@ChellyAhmed
Copy link
Contributor Author

Hi @mdo , thank you for your comment.
After reading your comment, I applied modifications to my code. I used the utility API, and updated the documentation accordingly. However, after I pushed, I realized that the utilities part of the code is very similar to the code in the PR #38167 by @oraliahdz . Thus, I want to double-check if I understood your feedback accurately.

@ChellyAhmed
Copy link
Contributor Author

I also think it is important to mention the test that failed.
It is about the max file size in the Bundelwatch check. The size of the generated utilities files exceeds the max file size for the check, as indicated in the screenshot below. However, I am not sure if it is possible to apply these modifications to the file without exceeding the max size limit. The file size is already big, and the modifications I added are not very significant.
Or is there a problem with the way I implemented it?
image

@julien-deramond
Copy link
Member

Don't worry about the failed Bundlewatch test. The difference is not that important, we can still increase the limit when the review will be over.

@ChellyAhmed ChellyAhmed requested a review from mdo May 6, 2023 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transition and animation helpers/utilities
3 participants