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

Range component ticks fix #438

Merged

Conversation

itaiperi
Copy link
Contributor

This PR fixes #437.

In a nutshell:

  • The problem - the Range component ticks math is wrong, only takes into account range [0, 100] and no step sizes < 1. Also, ticks are always displayed when step is provided, whether you want to or not.
  • The suggested fix - add displayTicks and ticksStep parameters to control when to display ticks, and what the ticks step size should be, which can now be different than step (before, they were the same by design). Also, take into account props.min and props.max to allow for the range the user intended in the ticks.

Copy link

netlify bot commented Dec 30, 2023

👷 Deploy Preview for react-daisyui processing.

Name Link
🔨 Latest commit c307925
🔍 Latest deploy log https://app.netlify.com/sites/react-daisyui/deploys/65906fadf9ee9c0008c126d1

Copy link

netlify bot commented Dec 30, 2023

Deploy Preview for react-daisyui ready!

Name Link
🔨 Latest commit f9fe9a5
🔍 Latest deploy log https://app.netlify.com/sites/react-daisyui/deploys/65907d819041bd000894a877
😎 Deploy Preview https://deploy-preview-438--react-daisyui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 30, 2023

👷 Deploy Preview for react-daisyui processing.

Name Link
🔨 Latest commit c307925
🔍 Latest deploy log https://app.netlify.com/sites/react-daisyui/deploys/65906fadf9ee9c0008c126d1

@itaiperi
Copy link
Contributor Author

Also, removed usage of useMemo, as there's no actual use for it, and the computation is fairly simple

Copy link
Collaborator

@benjitrosch benjitrosch left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the fix @itaiperi!

@benjitrosch benjitrosch merged commit e6f46a3 into daisyui:main Jan 1, 2024
4 checks passed
This was referenced Jan 1, 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.

Range component UI issues
2 participants