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

Fixes checkbox not working on iPhone #6

Merged
merged 1 commit into from
Aug 16, 2017
Merged

Conversation

gatanaso
Copy link

@gatanaso gatanaso commented Aug 14, 2017

Fixes #5
Fixes checkbox not working on iPhone.


This change is Reviewable

@gatanaso gatanaso changed the title Fixes checkbox issue#5 Fixes checkbox not working on iPhone Aug 14, 2017
@limonte
Copy link

limonte commented Aug 15, 2017

Is it possible to prove the bug with the test?


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@yuriy-fix
Copy link
Contributor

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


vaadin-checkbox.html, line 210 at r1 (raw file):

        _handleClick(e) {
          if (!this.disabled && !this.indeterminate) {
            if (e.composedPath()[0] !== this.$.label.querySelector('#nativeCheckbox')) {

Is it possible to replace this.$.label.querySelector('#nativeCheckbox') with this.$.nativeCheckbox?


Comments from Reviewable

@gatanaso
Copy link
Author

I was unable to prove the bug with a test on iPhone.


Review status: all files reviewed at latest revision, 1 unresolved discussion.


vaadin-checkbox.html, line 210 at r1 (raw file):

Previously, YuriyVaadin (Yuriy Yevstihnyeyev) wrote…

Is it possible to replace this.$.label.querySelector('#nativeCheckbox') with this.$.nativeCheckbox?

Yes, good point. I will update and push the changes. Thanks!


Comments from Reviewable

@gatanaso gatanaso force-pushed the fix/checkbox-on-iphone branch from 8266bcd to ff8c124 Compare August 16, 2017 12:01
@limonte
Copy link

limonte commented Aug 16, 2017

I was unable to prove the bug with a test on iPhone.

For this particular case test could be skipped IMO.

:lgtm:


Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@yuriy-fix
Copy link
Contributor

:lgtm:


Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

@yuriy-fix yuriy-fix merged commit ba4ff6c into master Aug 16, 2017
@jouni jouni deleted the fix/checkbox-on-iphone branch August 25, 2017 13:10
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.

4 participants