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

iOS fix for role="button" elements; [fixes #15935] #15947

Merged
merged 1 commit into from Mar 2, 2015
Merged

iOS fix for role="button" elements; [fixes #15935] #15947

merged 1 commit into from Mar 2, 2015

Conversation

patrickhlauke
Copy link
Member

Fixes "clickability" issue (and more generally, the firing of events such as focus as well) for traditionally non-focusable elements with role="button" (see https://developer.mozilla.org/en-US/docs/Web/Events/click#Safari_Mobile)

Fixes #15935

Fixes "clickability" issue (and more generally, the firing of events
such as focus as well) for traditionally non-focusable elements with
role="button" (see
https://developer.mozilla.org/en-US/docs/Web/Events/click#Safari_Mobile)

Fixes #15935
@patrickhlauke
Copy link
Member Author

/cc @mdo

@cvrebert
Copy link
Collaborator

cvrebert commented Mar 1, 2015

I'm 👍 on this.

@slaneyrw
Copy link

slaneyrw commented Mar 2, 2015

Carousel and Tab also suffer from this.

We added

[data-toggle="collapse"], [data-toggle="tab"] {
  cursor: pointer;
}

to our site less. Can you add this to the pull request, otherwise I will create my own

@cvrebert
Copy link
Collaborator

cvrebert commented Mar 2, 2015

@slaneyrw You meant Collapse, not Carousel, right?

@cvrebert
Copy link
Collaborator

cvrebert commented Mar 2, 2015

@slaneyrw Both of those cases involve anchors with hrefs, so they should (and AFAICT do) work fine as-is, unless your markup is wrong or something.

@patrickhlauke
Copy link
Member Author

In order to be accessible though, the collapse toggles should be <button> elements or be given a role="button" (covered by this PR), and the tab toggles should be <a href...>. Wonder if adding those extra cursor:pointer directives won't simply encourage bad usage...("hey, it works on my phone, so what's the deal?")

@cvrebert
Copy link
Collaborator

cvrebert commented Mar 2, 2015

the tab toggles should be <a href...>

They already are.

@slaneyrw
Copy link

slaneyrw commented Mar 2, 2015

Yes..lol. collapse is used as the data-toggle value
On 02/03/2015 8:59 PM, "Chris Rebert" notifications@github.com wrote:

@slaneyrw https://github.com/slaneyrw You meant Collapse, not Carousel,
right?


Reply to this email directly or view it on GitHub
#15947 (comment).

@patrickhlauke
Copy link
Member Author

They already are.

what i meant was: if you follow our examples, you should already be using <a href=...>, which has no clickability/focusability problem in iOS. but yeah, i'm tending to say no to the extra CSS and just keep this PR focused on the role[button] scenario only.

patrickhlauke added a commit that referenced this pull request Mar 2, 2015
@patrickhlauke patrickhlauke merged commit 8e6934c into twbs:master Mar 2, 2015
@cvrebert cvrebert mentioned this pull request Mar 2, 2015
@XhmikosR
Copy link
Member

XhmikosR commented Mar 2, 2015

patrickhlauke added a commit that referenced this pull request Mar 2, 2015
@cvrebert
Copy link
Collaborator

cvrebert commented Mar 5, 2015

cvrebert added a commit that referenced this pull request Apr 22, 2015
cvrebert added a commit that referenced this pull request Oct 25, 2015
Refs #15947
Somehow c256aca got lost during the SCSS translation...

[skip sauce]
cvrebert added a commit that referenced this pull request Nov 13, 2015
Refs #15947, #17542

[ci skip]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Popover with data-trigger="focus" does not work on iOS Safari when used on <a> without class="btn"
4 participants