You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Our company heavily uses web components to implement the micro-frontend architecture. Bootstrap-vue is heavily used in our application and we have thus far had to not use tooltips/popovers due to the shadow dom issue. After researching it internally for a while we realized a small adjustment to how the element is checked for DOM attachment and visibility would fix this issue. The "node.isConnected" property is widely supported by browsers and is ShadowDOM aware. The utility methods were created though due to specifically IE11 not supporting that property allowing us to be backward compatible. Wanted to contribute that back to the wider community.
Added to dom utility methods
isConnectedToDOM() checks if a target element is in the DOM and will check both Shadow and Regular DOM
getShadowRootOrRoot() will return the target's root either the Shadow Root or DOCUMENT.body
Updated isVisibile() dom util to use new isConnectedToDOM() function
Updated the dom.spec.js unit tests for the two new dom utilities
Fixed the dom.spec.js to get the select() and selectAll() tests working
PR checklist
What kind of change does this PR introduce? (check at least one)
Bugfix (fixes a boo-boo in the code) - fix(...), requires a patch version update
Feature (adds a new feature to BootstrapVue) - feat(...), requires a minor version update
Enhancement (augments an existing feature) - feat(...), requires a minor version update
ARIA accessibility (fixes or improves ARIA accessibility) - fix(...), requires a patch or minor version update
Documentation update (improves documentation or typo fixes) - chore(docs), requires a patch version update
Other (please describe)
Does this PR introduce a breaking change? (check one)
No
Yes (please describe since breaking changes require a minor version update)
The PR fulfills these requirements:
It's submitted to the dev branch, not the master branch
When resolving a specific issue, it's referenced in the PR's title (i.e. [...] (fixes #xxx[,#xxx]), where "xxx" is the issue number)
It should address only one issue or feature. If adding multiple features or fixing a bug and adding a new feature, break them into separate PRs if at all possible.
The title should follow the Conventional Commits naming convention (i.e. fix(alert): not alerting during SSR render, docs(badge): update pill examples, chore(docs): fix typo in README, etc.). This is very important, as the CHANGELOG is generated from these messages, and determines the next version type (patch or minor).
If new features/enhancement/fixes are added or changed:
Includes documentation updates
Includes component package.json meta section updates (prop, slot and event changes/updates)
Includes any needed TypeScript declaration file updates
New/updated tests are included and passing (required for new features and enhancements)
Existing test suites are passing
CodeCov for patch has met target (all changes/updates have been tested)
The changes have not impacted the functionality of other components or directives
ARIA Accessibility has been taken into consideration (Does it affect screen reader users or keyboard only users? Clickable items should be in the tab index, etc.)
If adding a new feature, or changing the functionality of an existing feature, the PR's
description above includes:
A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)
Added to dom utility methods
- isConnectedToDOM() checks if a target element is in the DOM and will check both Shadow and Regular DOM
- getShadowRootOrRoot() will return the target's root either the Shadow Root or DOCUMENT.body
Updated isVisibile() dom util to use new isConnectedToDOM() function
Updated the dom.spec.js unit tests for the two new dom utilities
Fixed the dom.spec.js to get the select() and selectAll() tests working
Merging #6997 (752661a) into dev (fab4161) will decrease coverage by 0.01%.
The diff coverage is 84.61%.
❗ Current head 752661a differs from pull request most recent head 5c169cc. Consider uploading reports for the commit 5c169cc to get more accurate results
Updated tests to get to 100% coverage of changed code + added coverage for isVisible()
Tightened the check in the getShadowRootOrRoot() method for document root
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe the PR
Our company heavily uses web components to implement the micro-frontend architecture. Bootstrap-vue is heavily used in our application and we have thus far had to not use tooltips/popovers due to the shadow dom issue. After researching it internally for a while we realized a small adjustment to how the element is checked for DOM attachment and visibility would fix this issue. The "node.isConnected" property is widely supported by browsers and is ShadowDOM aware. The utility methods were created though due to specifically IE11 not supporting that property allowing us to be backward compatible. Wanted to contribute that back to the wider community.
Added to dom utility methods
Updated isVisibile() dom util to use new isConnectedToDOM() function
Updated the dom.spec.js unit tests for the two new dom utilities
Fixed the dom.spec.js to get the select() and selectAll() tests working
PR checklist
What kind of change does this PR introduce? (check at least one)
fix(...), requires a patch version updatefeat(...), requires a minor version updatefeat(...), requires a minor version updatefix(...), requires a patch or minor version updatechore(docs), requires a patch version updateDoes this PR introduce a breaking change? (check one)
The PR fulfills these requirements:
devbranch, not themasterbranch[...] (fixes #xxx[,#xxx]), where "xxx" is the issue number)fix(alert): not alerting during SSR render,docs(badge): update pill examples,chore(docs): fix typo in README, etc.). This is very important, as theCHANGELOGis generated from these messages, and determines the next version type (patch or minor).If new features/enhancement/fixes are added or changed:
If adding a new feature, or changing the functionality of an existing feature, the PR's
description above includes: