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

5.2.0: Popover - Unable to use getInstance on .popover element #36837

Closed
3 tasks done
leettaylor opened this issue Jul 25, 2022 · 7 comments
Closed
3 tasks done

5.2.0: Popover - Unable to use getInstance on .popover element #36837

leettaylor opened this issue Jul 25, 2022 · 7 comments

Comments

@leettaylor
Copy link

Prerequisites

Describe the issue

Regression from v5.1.3. Can no longer get the Popover instance from the .popover element itself.

e.g.

const popover = bootstrap.Popover.getInstance('.popover');

// popover will be null

it now only works when using it on the trigger, e.g.

const popover = bootstrap.Popover.getInstance('.popover-trigger');

// popover will contain the popover element

Reduced test cases

v5.2.0: https://stackblitz.com/edit/h7ep9y?file=index.html,index.js
v5.1.3: https://stackblitz.com/edit/h7ep9y-vtddun?file=index.html

What operating system(s) are you seeing the problem on?

Linux

What browser(s) are you seeing the problem on?

Chrome

What version of Bootstrap are you using?

v5.2.0

@it-can
Copy link

it-can commented Jul 25, 2022

I have a similar problem with the Carousel. My fix was using getOrCreateInstance

@GeoSot
Copy link
Member

GeoSot commented Jul 26, 2022

In both cases, you are initializing the popover Instance using the following script

  document.querySelectorAll('[data-bs-toggle="popover"]').forEach((popover) => {
    new bootstrap.Popover(popover);
  });

So, programmatically thinking if you want to use the methods of this instance, you have to keep it as a variable or to refer to it, using the same selector

bootstrap.Popover.getInstance('[data-bs-toggle="popover"]');

Of course, you are right saying that you had found out a trick in where you could use the dynamic created element just to refer to the same instance, but as it was an intrusive and not documented way, I suppose you have to avoid it.
Worst case scenario, you can travel to the initial instance element using something like:

const x= document.querySelector(`.popover`).getAttribute('id')

const initialElem = document.querySelector('[aria-describedby="'+ x +'"]')

/cc @julien-deramond any different opinion on this?

@leettaylor
Copy link
Author

Of course, you are right saying that you had found out a trick in where you could use the dynamic created element just to refer to the same instance, but as it was an intrusive and not documented way, I suppose you have to avoid it.

I think updating my code to use the documented method that you've suggested makes the most sense.

Is it worth updating the migration guide to mention the removal of the undocumented way?

@GeoSot
Copy link
Member

GeoSot commented Jul 29, 2022

Is it worth updating the migration guide to mention the removal of the undocumented way?

I think is controversial, to document (mention) a change on an undocumented way 😉

@GeoSot GeoSot closed this as completed Jul 29, 2022
@Rezyan
Copy link

Rezyan commented Jul 31, 2022

Of course, you are right saying that you had found out a trick in where you could use the dynamic created element just to refer to the same instance, but as it was an intrusive and not documented way, I suppose you have to avoid it. Worst case scenario, you can travel to the initial instance element using something like:

const x= document.querySelector(`.popover`).getAttribute('id')

const initialElem = document.querySelector('[aria-describedby="'+ x +'"]')

/cc @julien-deramond any different opinion on this?

Hi @GeoSot, should I use the way you described to migrate my Bootstrap v5.1 code to Bootstrap v5.2 code? I need to close all open tooltips and popovers.

This is my current code:

$('.tooltip').tooltip('hide'); // Close all open tooltips.
$('.popover').popover('hide'); // Close all open popovers.

@GeoSot
Copy link
Member

GeoSot commented Aug 1, 2022

Something like ???

  document.querySelectorAll('[data-bs-toggle="popover"][aria-describedby]').forEach((popover) => {
    bootstrap.Popover.getOrCreateInstance(popover).hide();
  });

(same for tooltip)

@Rezyan
Copy link

Rezyan commented Aug 1, 2022

That's what I thought, thank you @GeoSot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

No branches or pull requests

5 participants