Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Popover directive doesn't work with Safari when using popover-trigger="focus" #3687

Closed
kpx-dev opened this issue May 18, 2015 · 29 comments
Closed

Comments

@kpx-dev
Copy link

kpx-dev commented May 18, 2015

Hi guys,

Ex: this works on chrome and firefox, but not on Safari (8.0.6). I'm on Mac OS X Yosemite
< a href="" popover="hola!" popover-trigger="focus">Hello!< / a >

When in Safari, when I click on that link, nothing shows up.

Thanks!

@rvanbaalen
Copy link
Contributor

Please provide a plunkr reproducing your problem.

@rvanbaalen rvanbaalen added this to the Backlog milestone May 19, 2015
@kpx-dev
Copy link
Author

kpx-dev commented May 19, 2015

@RobJacobs
Copy link
Contributor

I've found focus to be flaky in Safari, does it work on a click when you switch to a click trigger? Does it work when you tab onto the button with a focus trigger?

_edit_
I think your best bet is to use the $tooltipProvider to add your own trigger that listens for click and focus like so:

$tooltipProvider.setTriggers({ 'click focus': 'mouseleave blur' });

Then implement like so:

<button class="btn btn-default" tooltip="I am a tooltip" tooltip-trigger="click focus">Tooltip</button>

@kpx-dev
Copy link
Author

kpx-dev commented May 19, 2015

@RobJacobs awesome, your suggestion work perfectly! Thanks.

@rvanbaalen
Copy link
Contributor

Glad you have a workaround. That still leaves this as a bug for Safari though, right @RobJacobs ?

@RobJacobs
Copy link
Contributor

I don't think it's a bug in our code, Safari just doesn't seem to propagate the focus event from a click event. I didn't see any preventDefault or stopPropagation in the code either.

@rvanbaalen
Copy link
Contributor

It's unfortunate that references like this one aren't up to date with the newest browsers.

@kpx-dev
Copy link
Author

kpx-dev commented May 19, 2015

Maybe a fix is to document such workaround for Safari at the Tooltip Set Trigger section?
http://angular-ui.github.io/bootstrap/#/tooltip

@chrisirhc
Copy link
Contributor

See discussion at Twitter Bootstrap: twbs/bootstrap#11788
Indeed click focus as the trigger is the workaround specified.

@wesleycho
Copy link
Contributor

We should probably put a workaround in UI Bootstrap then, even if Safari's buggy behavior is the culprit in this case.

@Astray-git
Copy link

Firefox on OSX has the same problem

@sethflowers
Copy link

+1

@icfantv
Copy link
Contributor

icfantv commented Sep 18, 2015

Can anyone confirm whether or not 1) this is still an issue and 2) if Apple intends on releasing a new version of Safari with El Capitan? And if so, is anyone running the beta that can test to see if this is still an issue? Thanks.

@wesleycho
Copy link
Contributor

Even if they do, we should still investigate this - Safari 7 is still the most used version of Safari out in the wild, it may take a while before we see this fixed at the browser level.

@sethflowers
Copy link

Just confirmed in saucelabs that it does not work in El Capitan with Safari 8.1. I used the following plunk: http://plnkr.co/edit/ZDdiKt1VvQVAwxwblwvW?p=preview

@icfantv
Copy link
Contributor

icfantv commented Sep 18, 2015

@wesleycho, agreed.

I'm a little confused. Is the fix this or is it this? @chrisirhc, do you remember?

@wesleycho, what type of workaround do we want? something like:

if (isMac && (browser === safari) && (version >= 7)) {
  // stuff
}
else {
  // other stuff
}

please note that I had to hold my nose while typing that...

@RobJacobs
Copy link
Contributor

From TWBS PR #141456 seems the fix is to use

<a tabindex="0">  

instead of

<button> 

Relevant links on MDN:

button

anchor

So I would say there is nothing to change in the library and it's an implementation concern. If you want to use the focus trigger, use an a element with a tabindex

@icfantv
Copy link
Contributor

icfantv commented Sep 18, 2015

Nice @RobJacobs. I'm ok with this being a wontfix then and adding comments to both the FAQs and popover/tooltip docs with a Nota Bene for OSX Safari 7.0+ users. @wesleycho, you cool with that?

@wesleycho
Copy link
Contributor

I'm fine with that resolution, although perhaps @Foxandxss might have something to say about the docs

@Foxandxss
Copy link
Contributor

Yeah, it is always good to document this stuff.

If I understood correctly, OSX safari users needs to use an element with a tagindex if they want to use popover-trigger with focus, right? Does that affect tooltip as well?

@Foxandxss
Copy link
Contributor

Alright, will create a PR to fix this via doc.

@wesleycho
Copy link
Contributor

@Foxandxss don't forget about this :) .

@Foxandxss
Copy link
Contributor

Wari wari! Will write it down on my todo list.

@wesleycho
Copy link
Contributor

Looks like this is already in the docs from 4e60e22 - closing as resolved.

@QuaoQuo
Copy link

QuaoQuo commented Dec 21, 2015

html:
<a href="javascript:void(0)" class="popupover" data-toggle="popover" data-trigger="click" title="Popover" data-content="something here"></a>

JS:
$('.popupover').popover();
jQuery("body").on("click touchstart", '.popupover', function() {
$(this).popover("show");
$('.popupover').not(this).popover("hide"); // hide other popovers
return false;
});
jQuery("body").on("click touchstart", function() {
$('.popupover').popover("hide"); // hide all popovers when clicked on body
});

@wesleycho
Copy link
Contributor

We do not support the Twitter Bootstrap JS @QuaoQuo - please do not comment on closed issues for technical support, GitHub issues are not the place for that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants