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.

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

feat: add prop to support reversed animation #50

Merged
merged 3 commits into from
Oct 23, 2024

Conversation

vh-x
Copy link
Contributor

@vh-x vh-x commented Jul 8, 2024

This PR introduces a new prop for reversed animation. When set to true, the slot machine animation will render from right to left instead of the default left to right.

  • Added reversed prop to Props interface
  • Updated the animation logic to respect the reversed prop
  • Default value for reversed is false

@vh-x vh-x force-pushed the feat/reversed-animation branch from 69b9029 to e8b2b8d Compare July 9, 2024 04:49
@almond-bongbong
Copy link
Owner

Thank you for your contribution. I will review the changes and provide feedback soon. Thank you.

@almond-bongbong
Copy link
Owner

First and foremost, I sincerely apologize for the delay in reviewing this PR.

Thank you for adding this valuable feature. The ability to control the animation direction is an excellent improvement that will enhance the component's flexibility.

If I may, I'd like to respectfully suggest considering some alternatives for the reversed prop name:

  1. The term reversed might be slightly ambiguous, as it's not immediately clear what aspect is being reversed.

  2. To further improve code readability and maintainability, perhaps we could consider a more explicit boolean prop name. Here are a few suggestions for your consideration:

    • animateFromEnd: boolean
    • reverseAnimationOrder: boolean
    • startFromLastDigit: boolean

These alternatives aim to more clearly convey the prop's purpose while maintaining its boolean nature.

Additionally, it would be greatly appreciated if you could update the props list table in the README to include this new prop. This will ensure that the documentation remains comprehensive and up-to-date for all users.

Once again, thank you for this great enhancement. With a slightly more descriptive prop name and updated documentation, it will be an excellent addition to the component.

@vh-x vh-x force-pushed the feat/reversed-animation branch from e8b2b8d to 1d62c2e Compare September 8, 2024 07:51
@vh-x
Copy link
Contributor Author

vh-x commented Sep 8, 2024

Thank you for your feedback!

I agree with your point, and I have renamed the prop to startFromLastDigit as suggested.

The README has also been updated accordingly. I appreciate your kind words and guidance, and I’m glad this feature will add value to the component.

@almond-bongbong
Copy link
Owner

Thank you for your contribution

@almond-bongbong almond-bongbong merged commit 9c90fed into almond-bongbong:main Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants