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

Store a reference to the tooltip/popover on insert #766

Merged
merged 5 commits into from
Aug 29, 2023
Merged

Conversation

cpsievert
Copy link
Collaborator

Closes #765

…t we can more reliably use it to restore when hidden
);
}
const body = tip.querySelector(".popover-body");
const el = this.bsPopoverEl;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we unset this element at some point? Or do we just update it when a new popover is inserted?

Copy link
Collaborator Author

@cpsievert cpsievert Aug 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, good point. Addressed in cc0ac46

Copy link
Member

@gadenbuie gadenbuie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still left with the general feeling that the references we should track in the class instance should be to elements that we control, i.e. the popover body and title. But I also understand that this may be a quicker and easier way to solve the current bug. I'm happy if we just make a note to think about this again if we end up in this area again.

@cpsievert
Copy link
Collaborator Author

cpsievert commented Aug 29, 2023

I'm still left with the general feeling that the references we should track in the class instance should be to elements that we control, i.e. the popover body and title.

Yea, I had another quick look into this. That approach would definitely simplify some logic and make it more robust to changes to Bootstrap. The downside though is that, if we don't reach into the tooltip/popover element (to return the contents to their rightful place), then we'd open ourselves up to restoring "stale" content, which is a problem for popovers especially

@cpsievert cpsievert merged commit e34e460 into main Aug 29, 2023
1 check passed
@cpsievert cpsievert deleted the bs5.3-tooltip-fix branch August 29, 2023 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JS error occurs when a tooltip/popover is hidden
2 participants