Skip to content

Slider/add throttleTime prop #3020

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

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

@liatnetach liatnetach requested a review from ethanshar April 8, 2024 14:36
@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 requested review from Inbal-Tish April 9, 2024 11:32
@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
@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.

3 participants