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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Checkbox] Bug fixes 馃敟 #672

Merged
merged 7 commits into from May 31, 2021
Merged

[Checkbox] Bug fixes 馃敟 #672

merged 7 commits into from May 31, 2021

Conversation

jjenzz
Copy link
Contributor

@jjenzz jjenzz commented May 28, 2021

Cleans this up a bit to use the same BubbleInput approach that Slider uses and fixes a bunch of bugs in the process.

CleanShot.2021-05-28.at.18.11.13.mp4

Accessibility

Screen.Recording.2021-05-31.at.13.06.14.mov

Interesting that our custom checkbox with custom label also improves cursor position accuracy in VoiceOver.

Copy link
Collaborator

@benoitgrelard benoitgrelard left a comment

Choose a reason for hiding this comment

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

There seems to be some issues with this.

If I have:

<div onClick={() => alert('click bubble')}>
      <Checkbox className={rootClass}>
        <CheckboxIndicator className={indicatorClass} />
      </Checkbox>
    </div>

It happens straight on page load, and then also it doesn't seem to have resolved the double event that we see in #667

@jjenzz jjenzz marked this pull request as draft May 29, 2021 16:20
@jjenzz jjenzz marked this pull request as ready for review May 31, 2021 12:30
@jjenzz
Copy link
Contributor Author

jjenzz commented May 31, 2021

@benoitgrelard This should be ready for a review now 馃檹

@benoitgrelard benoitgrelard changed the title [Checkbox] Bug fixes [Checkbox] Bug fixes 馃敟 May 31, 2021
packages/react/label/src/Label.tsx Show resolved Hide resolved
packages/react/checkbox/src/Checkbox.tsx Outdated Show resolved Hide resolved
packages/react/checkbox/src/Checkbox.tsx Show resolved Hide resolved
packages/react/checkbox/src/Checkbox.tsx Outdated Show resolved Hide resolved
packages/react/checkbox/src/Checkbox.tsx Show resolved Hide resolved
packages/react/checkbox/src/Checkbox.tsx Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants