Fix for opening accordion item on iOS devices

[Techn1x]

Thanks to @sukima for the fix

Before the ember-bootstrap beta.1 release, on iOS devices (or at least, iOS 10 anyway), clicking the accordion item anywhere didn't open the accordion.

On beta.1, it was now possible to click the title to open the accordion item (but clicking the accordion item itself still did not work.) This was likely due to this fix #311

It turns out, that iOS devices refuse to click on elements that aren't <button> or <a>. To allow clicking on other elements (such as <div>) need to apply the style cursor: pointer; to the element - doing this fixes the issue on iOS.

Should this style/class be added by default to accordion items so that they work on iOS? I can probably to a PR if needed

[srvance]

Thanks for filing this, @Techn1x. A PR would be great. Any opinion on this beforehand, @simonihmig?

[sukima]

References

• https://stackoverflow.com/questions/15095868/jquery-click-not-working-in-ios
• https://www.quirksmode.org/blog/archives/2014/02/mouse_event_bub.html
• onClick broken on iOS. facebook/react#134
• https://css-tricks.com/snippets/css/give-clickable-elements-a-pointer-cursor/
• http://www.shdon.com/blog/2013/06/07/why-your-click-events-don-t-work-on-mobile-safari

[Techn1x]

I haven't done a lot of PR's on this project, what would be the best way to do this?

I went to add this:

/* iOS fix - elements aren't clickable without pointer */
  attributeBindings: ['style'],
  style: 'cursor:pointer',

To the base bs-accordion item component here:
https://github.com/kaliber5/ember-bootstrap/blob/master/addon/components/base/bs-accordion/item/title.js#L14-L37

Which works, but got a warning
WARNING: Binding style attributes may introduce cross-site scripting vulnerabilities;

Not really sure where to define classes for this addon either

[sukima]

You can get around that with this:

style: 'cursor:pointer'.htmlSafe()

But this is still dirty (and not something I would expect in an addon) instead I would leverage the component's class and add it into the CSS. What confuses me is why the Bootstrap CSS that this addon imports does not already have this in their accordion styles.

[srvance]

@sukima Is this BS3 or BS4? They're not really doing anything on BS3 now and this may not have been a behavior they anticipated when BS3 was under development. If it still exists in BS4, it should be an issue against twbs/bootstrap while they're still bringing it to fruition.

[sukima]

[srvance]

What do you see when you run ember bootstrap:info? Can you paste the results here?

[Techn1x]

Definitely BS3, not sure if issue is present in BS4.

I found a better fix for this. Inside native BS3 stylesheets there is this that I could use for accordion items;

// iOS "clickable elements" fix for role="button"
//
// 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

[role="button"] {
  cursor: pointer;
}

As for BS4, this discussion looks relevant;
https://www.bountysource.com/issues/41219012-remove-cursor-pointer

But I am not sure what is the best way to proceed. I can add role="button" to the base accordion item, the other solution seems to be to add an empty click handler to target or any ancestor

[sukima]

I can add role="button" to the base accordion item

This looks like the best solution IMHO. I would 👍 this.

[sukima]

role="button" also adds accessibility to the accordion component which is a win win!

[simonihmig]

Seems the original Bootstrap example does not support clicking the anywhere in the header, only on the <a>: http://getbootstrap.com/javascript/#collapse-example-accordion

The .panel-heading already has an ARIA role (role="tab") in line with the Bootstrap example. So adding role="button" would not be appropriate, I think?

Maybe we should add a app/styles/app.css file and put the cursor: pointer fix there, so this get's added to the apps styles automatically?

[Techn1x]

Hmm, in terms of UI and usability, that's a strange choice by bootstrap.... make something look like a button, but actually only the title acts like a button. I definitely prefer the approach of making the whole thing a button/tab.

So what's the best approach here, a more generic style like

[role="tab"] {
    cursor: pointer;
}

Or something specific to just accordions (would require making a new class, I think?)

.accordion-heading {
    cursor: pointer;
}

[sukima]

I like:

.panel-heading[role='tab'] {
  cursor: pointer;
}

And this should be in Bootstrap prime. Seriously.

[simonihmig]

I added a PR that implements the fix that @sukima suggested: #369

Does that look ok for everybody? cc @Techn1x