-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Simplifies Toggle's switch-variant to not require measuring logic. Adds icon customization to Toggle's checkbox-variant. #2315
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
Conversation
…entage values for the switch knob, simplifying the implementation.
frankcalise
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, added a couple of minor questions
Hey boss, I dont see any comments. |
| * Default: "checkbox" | ||
| */ | ||
| variant?: Variants | ||
| variant?: unknown |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this go away since provided in each of the 3 types making up the discriminating union? Genuinely asking for my own understanding as I was trying to write some dynamic props earlier in the week
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have this here for the description mainly. If I add descriptions to the 3 Types, then the documentation for the selected discriminating prop will be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ok cool thanks
Weird, I must have not hit Start a review before - I added them back in |
## [8.4.2](v8.4.1...v8.4.2) (2022-11-21) ### Bug Fixes * **boilerplate:** Simplifies Toggle's switch-variant to not require measuring logic. Adds icon customization to Toggle's checkbox-variant. ([#2315](#2315) by [@yulolimum](https://github.com/yulolimum)) ([c6181c4](c6181c4))
|
🎉 This PR is included in version 8.4.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Please verify the following:
yarn testjest tests pass with new tests, if relevantREADME.mdhas been updated with your changes, if relevantDescribe your PR
@robinheinze reported that the app she is working on has a weird screen offset when the Toggle is used. The reason for that is this issue: software-mansion/react-native-reanimated#3368
The Toggle switch-variant uses reanimated's
FadeIn. This was added to give the switch time to calculate the knob width to determine its position along the track. This was needed in case the component overrides specified a percentage width for the knob.I think it's reasonable to only allow point values for knob width customization. This cleans up the switch code and remove the need for measuring, which in effect, removes the need for the FadeIn.
This PR also separates the Types for the different Toggle variants for better prop hover descriptions and prop matches.
This PR also adds the ability to customize the checkbox-variant icon.