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

Slider/add throttleTime prop #3020

Merged
merged 3 commits into from Apr 9, 2024
Merged

Conversation

liatnetach
Copy link
Contributor

@liatnetach liatnetach commented Apr 8, 2024

Description

Add throttleTime prop in Slider component

Changelog

Add throttleTime prop in Slider component

Additional info

@ethanshar ethanshar added the Important for Next Release PR that must be included in the release version label Apr 9, 2024
@@ -278,10 +283,22 @@ const Slider = React.memo((props: Props) => {
}
}, 200), [onValueChange]);

const _onValueChange = useCallback((value: number) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not pass the throttle wait (200/100 to 0) instead of having code duplications?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Inbal-Tish
I talked with Liat about it already, right now even without the throttle it doesn't work so good..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added throttleTime prop instead 🙏

@liatnetach liatnetach changed the title Slider/add disableThrottling prop Slider/add throttleTime prop Apr 9, 2024
@ethanshar ethanshar merged commit 0157466 into master Apr 9, 2024
1 check passed
@ethanshar ethanshar deleted the slider/add-disable-throttling-prop branch April 9, 2024 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Important for Next Release PR that must be included in the release version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants