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

Tooltip selector regression in v3.3.0 #15168

Closed
rymohr opened this issue Nov 18, 2014 · 8 comments · Fixed by #15466
Closed

Tooltip selector regression in v3.3.0 #15168

rymohr opened this issue Nov 18, 2014 · 8 comments · Fixed by #15466
Labels
Milestone

Comments

@rymohr
Copy link

rymohr commented Nov 18, 2014

The selector handling changes introduced in 3.3.0 break manual tooltips initialized with a selector.

http://jsfiddle.net/sk5o25b9/ (worked fine in 3.2.0)
http://jsfiddle.net/sk5o25b9/1 (busted in 3.3.0+)

See PR #14189 and related commit 1b32376
Related to #14167

@cvrebert cvrebert added the js label Nov 18, 2014
@cvrebert cvrebert added this to the v3.3.2 milestone Nov 18, 2014
@mattlunn
Copy link

Youch. The change you reference prevents any "use" of the tooltip/ popup after you've initialized it; e.g you can't pass a string to $().popover() or $().tooltip() anymore (e.g. $().popover('destroy').

A solution (!?) would be to alter the signature of Popover(option) and Tooltip(option) to be Popover([selector], option) and Tooltip([selector], option), e.g.:

$('foo').popover({
  selector: 'mark',
  trigger: 'hover',
  container: "body",
});

// later...
$('foo').popover('mark', 'destroy');

This would then give Tooltip() and Popover() functions enough context in later invocations to know which instance the user is trying to target. Obviously if you don't specify a "selector", the "selector" argument won't need to be provided in further calls, and you could get away with $('foo').popover('destroy') directly.

See also http://stackoverflow.com/q/27108979/444991

redbmk added a commit to redbmk/bootstrap that referenced this issue Dec 5, 2014
@redbmk
Copy link
Contributor

redbmk commented Dec 5, 2014

I believe I have a fix for this: 465ef28

@redbmk
Copy link
Contributor

redbmk commented Dec 5, 2014

Nevermind...that seemed to have fixed it for tooltip selectors but broken it for tooltips without selectors. I'll see if I can find a more complete solution tomorrow.

@redbmk
Copy link
Contributor

redbmk commented Dec 6, 2014

I can't come up with a good, backwards-compatible way to fix this. I ran into this issue because I'm calling $().tooltip("destroy") where there is a selector, but @mattlunn's proposal would mean I would have to change my code to add the selector in there. It seems like it might be best to revert 1b32376 and look into a new way to handle selectors for v4.0 that fixes #14167.

@rymohr
Copy link
Author

rymohr commented Dec 8, 2014

Only thing I can think of would be to track the last selector given, and use that as the default for any future calls.

@cvrebert
Copy link
Collaborator

Maybe instead of storing the instance data per-selector, we could add an explicit instanceId (or whatever) option and store the data per-ID. And when no ID is provided, default to some special ID so that stuff not using selector still works.
Just a random thought.

@cvrebert
Copy link
Collaborator

@hnrch02 Any objection to reverting #14189 for the time being?

@hnrch02
Copy link
Collaborator

hnrch02 commented Dec 30, 2014

@cvrebert No, seems like this is a bigger issue than #14167. We'll see if we can address it in v4.

cvrebert added a commit that referenced this issue Dec 30, 2014
This reverts commit 1b32376.
This reverts PR #14189 because it caused major regressions.

Fixes #15168.

We'll try to revisit #14167's feature request in Bootstrap v4.

[skip validator]
cvrebert added a commit that referenced this issue Dec 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants