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

Custom trigger for tooltip doesn't seems to work on 0.13.4 #4364

Closed
cyrilgandon opened this issue Sep 7, 2015 · 10 comments
Closed

Custom trigger for tooltip doesn't seems to work on 0.13.4 #4364

cyrilgandon opened this issue Sep 7, 2015 · 10 comments
Assignees

Comments

@cyrilgandon
Copy link

I have used this demo with great success for setting custom trigger on tooltip:

http://plnkr.co/edit/0wEqzz?p=preview

From that post : #625 (comment)

It work until v0.13.3. Unfortunately, the release 0.13.4 of the 3 september broke the triggers:

http://plnkr.co/edit/KEGMhj2jbF7z6h2Qeu4i?p=preview

Ok never mind, I just read the doc:

"For any non-supported value, the trigger will be used to both show and hide the tooltip. Using the 'none' trigger will disable the internal trigger(s), one can then use the tooltip-is-open attribute exclusively to show and hide the tooltip."

So this becomes:
http://plnkr.co/edit/dm408dUoFKRVQKE9SOTb?p=preview

@cyrilgandon
Copy link
Author

even though their is a workaround, this should be log in the breaking changes of the releases notes.

https://github.com/angular-ui/bootstrap/blob/master/CHANGELOG.md

@dbkup
Copy link

dbkup commented Sep 8, 2015

I came to the same realization after debugging my code for this problem for the last ~3 hours.

@cyrilgandon I read that differently, I believe that this is still a bug, i.e. it should work with the custom triggers. The note for 'none' explains how to disable internal triggers, but this should not affect setTriggers which sets custom triggers.

Edit:
I glanced over the code in tooltip.js and there is still code in place which should handle triggers. Will take a look later, if there is time, to see what's going on.

@readelobdill
Copy link

+1

@readelobdill
Copy link

Hey fellers,

The reason this is happening is because of the change in #4060. The event listeners are being set up on the raw DOM elements so instead of elem.trigger('customEvent') we now need elem[0].dispatchEvent(new Event('customEvent))

@cyrilgandon workaround does work although it may be easier the change the event triggering for some folks here rather than using the tooltip-is-open attribute in conjunction with tooltip-trigger="none"

@icfantv
Copy link
Contributor

icfantv commented Sep 9, 2015

Hey guys, can any one of you please check to see if the fix for #4384 fixes this issue? Currently it's a PR that has not yet been merged.

If you want to check that quickly, I can merge it into master.

@dbkup
Copy link

dbkup commented Sep 10, 2015

@readelobdill 👍 thanks that works for me

@icfantv I copied over the bootstrap-ui.js from plunk used in #4384 into a fork of @cyrilgandon's plunk here.
It doesn't work with trigger or dispatchEvent... could be that I'm not doing it correctly, but the basic tooltip is showing.

@wesleycho
Copy link
Contributor

Seeing this description, this is indeed an issue still - I will work on fixing this.

@wesleycho wesleycho self-assigned this Sep 11, 2015
@wesleycho
Copy link
Contributor

Hmm, so this is a tricky issue - this was an unexpected breaking change that #4322 caused, as pointed out by @readelobdill. It was meant to address #4060. I apologize for this mistake, I misassessed the impact of the change.

Unfortunately due to the nature of the bug, I think using raw event listeners is necessary for a better UX here. I will update the changelog to reflect that this is a breaking change though & mention the workaround.

@forceuser
Copy link

Now after #4322 there is a new bug, events dont unbind, and 'hide' method can happen after scope is destroyed causing error : Uncaught TypeError: Cannot set property 'isOpen' of null

the reason is event still unbinds using jquery methods: https://github.com/wesleycho/bootstrap/blob/97a56634d9f5469fe40fb486898016ea09e7b59c/src/tooltip/tooltip.js#L382-L385

and also there no unbind for toggleTooltipBind

@wesleycho
Copy link
Contributor

The first half is fixed on master, the second half will be fixed soon (it is in another PR atm)

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

6 participants