From 752661a87affe6490579ec16052ed241a6b1fa4c Mon Sep 17 00:00:00 2001 From: Charles Rasico <547160+rasicoc@users.noreply.github.com> Date: Wed, 22 Jun 2022 08:48:15 -0500 Subject: [PATCH 1/2] fix(b-tooltip): Updated tooltip to work under shadowDOM 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 --- src/components/tooltip/helpers/bv-tooltip.js | 9 +- src/utils/dom.js | 19 +++- src/utils/dom.spec.js | 100 ++++++------------- 3 files changed, 56 insertions(+), 72 deletions(-) diff --git a/src/components/tooltip/helpers/bv-tooltip.js b/src/components/tooltip/helpers/bv-tooltip.js index 1f0d114ac82..ee6d58f5cc2 100644 --- a/src/components/tooltip/helpers/bv-tooltip.js +++ b/src/components/tooltip/helpers/bv-tooltip.js @@ -29,8 +29,10 @@ import { contains, getAttr, getById, + getShadowRootOrRoot, hasAttr, hasClass, + isConnectedToDOM, isDisabled, isElement, isVisible, @@ -262,7 +264,7 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ this.$nextTick(() => { const target = this.getTarget() - if (target && contains(document.body, target)) { + if (target && (target.isConnected || isConnectedToDOM(target))) { // Copy the parent's scoped style attribute this.scopeId = getScopeId(this.$parent) // Set up all trigger handlers and listeners @@ -420,7 +422,7 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ const target = this.getTarget() if ( !target || - !contains(document.body, target) || + !isConnectedToDOM(target) || !isVisible(target) || this.dropdownOpen() || ((isUndefinedOrNull(this.title) || this.title === '') && @@ -567,8 +569,9 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ getContainer() { // Handle case where container may be a component ref const container = this.container ? this.container.$el || this.container : false - const body = document.body const target = this.getTarget() + const body = getShadowRootOrRoot(target) + // If we are in a modal, we append to the modal, If we // are in a sidebar, we append to the sidebar, else append // to body, unless a container is specified diff --git a/src/utils/dom.js b/src/utils/dom.js index a48614e1a4c..4d25b7b7d09 100644 --- a/src/utils/dom.js +++ b/src/utils/dom.js @@ -83,7 +83,7 @@ export const isActiveElement = el => isElement(el) && el === getActiveElement() // Determine if an HTML element is visible - Faster than CSS check export const isVisible = el => { - if (!isElement(el) || !el.parentNode || !contains(DOCUMENT.body, el)) { + if (!isElement(el) || !el.parentNode || !isConnectedToDOM(el)) { // Note this can fail for shadow dom elements since they // are not a direct descendant of document.body return false @@ -100,6 +100,23 @@ export const isVisible = el => { return !!(bcr && bcr.height > 0 && bcr.width > 0) } +// used to grab either the shadow root in a web component or the main document body +export const getShadowRootOrRoot = el => { + if (el.getRootNode == null) { + return DOCUMENT.body + } + const root = el.getRootNode() + if (root.nodeType === 9) { + return root.body + } + return root +} + +export const isConnectedToDOM = el => { + // If node.isConnected undefined then fallback to IE11 compliant check + return el.isConnected == null ? contains(DOCUMENT.body, el) : el.isConnected +} + // Determine if an element is disabled export const isDisabled = el => !isElement(el) || el.disabled || hasAttr(el, 'disabled') || hasClass(el, 'disabled') diff --git a/src/utils/dom.spec.js b/src/utils/dom.spec.js index 0281a44858e..54e600560f4 100644 --- a/src/utils/dom.spec.js +++ b/src/utils/dom.spec.js @@ -3,9 +3,11 @@ import { closest, contains, getAttr, + getShadowRootOrRoot, getStyle, hasAttr, hasClass, + isConnectedToDOM, isDisabled, isElement, matches, @@ -25,28 +27,30 @@ const template = ` ` -const App = { template } +let App +let wrapper describe('utils/dom', () => { - it('isElement() works', async () => { - const wrapper = mount(App, { + beforeEach(() => { + App = { template } + wrapper = mount(App, { attachTo: document.body }) + }) + + afterEach(() => { + wrapper.destroy() + }) + it('isElement() works', async () => { expect(wrapper).toBeDefined() expect(wrapper.find('div.foo').exists()).toBe(true) expect(isElement(wrapper.element)).toBe(true) expect(isElement(null)).toBe(false) expect(isElement(App)).toBe(false) - - wrapper.destroy() }) it('isDisabled() works', async () => { - const wrapper = mount(App, { - attachTo: document.body - }) - expect(wrapper).toBeDefined() const $btns = wrapper.findAll('div.baz > button') @@ -55,15 +59,28 @@ describe('utils/dom', () => { expect(isDisabled($btns.at(0).element)).toBe(false) expect(isDisabled($btns.at(1).element)).toBe(false) expect(isDisabled($btns.at(2).element)).toBe(true) + }) - wrapper.destroy() + // NOTE: Need to figure out how to test against shadowDOM + it('isConnectedToDOM() Regular DOM', async () => { + expect(wrapper).toBeDefined() + + const $barspan = wrapper.findAll('span.barspan') + expect($barspan).toBeDefined() + expect($barspan.length).toBe(1) + expect(isConnectedToDOM($barspan.at(0).element)).toBe(true) }) - it('hasClass() works', async () => { - const wrapper = mount(App, { - attachTo: document.body - }) + it('getShadowRootOrRoot() Regular DOM', async () => { + expect(wrapper).toBeDefined() + + const $baz = wrapper.find('div.baz') + const $documentBody = getShadowRootOrRoot($baz.element) + expect($documentBody).toBeDefined() + expect($documentBody.toString()).toBe('[object HTMLBodyElement]') + }) + it('hasClass() works', async () => { expect(wrapper).toBeDefined() const $span = wrapper.find('span.barspan') @@ -73,15 +90,9 @@ describe('utils/dom', () => { expect(hasClass($span.element, 'foobar')).toBe(true) expect(hasClass($span.element, 'fizzle-rocks')).toBe(false) expect(hasClass(null, 'foobar')).toBe(false) - - wrapper.destroy() }) it('contains() works', async () => { - const wrapper = mount(App, { - attachTo: document.body - }) - expect(wrapper).toBeDefined() const $span = wrapper.find('span.barspan') @@ -95,15 +106,9 @@ describe('utils/dom', () => { expect(contains(wrapper.element, $btn1.element)).toBe(true) expect(contains($span.element, $btn1.element)).toBe(false) expect(contains(null, $btn1.element)).toBe(false) - - wrapper.destroy() }) it('closest() works', async () => { - const wrapper = mount(App, { - attachTo: document.body - }) - expect(wrapper).toBeDefined() const $btns = wrapper.findAll('div.baz > button') @@ -122,15 +127,9 @@ describe('utils/dom', () => { expect(closest('div.not-here', $btns.at(0).element)).toBe(null) expect(closest('div.baz', $baz.element)).toBe(null) expect(closest('div.baz', $baz.element, true)).toBe($baz.element) - - wrapper.destroy() }) it('matches() works', async () => { - const wrapper = mount(App, { - attachTo: document.body - }) - expect(wrapper).toBeDefined() const $btns = wrapper.findAll('div.baz > button') @@ -148,15 +147,9 @@ describe('utils/dom', () => { expect(matches($btns.at(0).element, 'div.bar > button')).toBe(false) expect(matches($btns.at(0).element, 'button#button1')).toBe(true) expect(matches(null, 'div.foo')).toBe(false) - - wrapper.destroy() }) it('hasAttr() works', async () => { - const wrapper = mount(App, { - attachTo: document.body - }) - expect(wrapper).toBeDefined() const $btns = wrapper.findAll('div.baz > button') @@ -169,15 +162,9 @@ describe('utils/dom', () => { expect(hasAttr($btns.at(2).element, 'disabled')).toBe(true) expect(hasAttr($btns.at(2).element, 'role')).toBe(false) expect(hasAttr(null, 'role')).toBe(null) - - wrapper.destroy() }) it('getAttr() works', async () => { - const wrapper = mount(App, { - attachTo: document.body - }) - expect(wrapper).toBeDefined() const $btns = wrapper.findAll('div.baz > button') @@ -193,15 +180,9 @@ describe('utils/dom', () => { expect(getAttr(null, 'role')).toBe(null) expect(getAttr($btns.at(0).element, '')).toBe(null) expect(getAttr($btns.at(0).element, undefined)).toBe(null) - - wrapper.destroy() }) it('getStyle() works', async () => { - const wrapper = mount(App, { - attachTo: document.body - }) - expect(wrapper).toBeDefined() const $span = wrapper.find('span.barspan') @@ -210,15 +191,9 @@ describe('utils/dom', () => { expect(getStyle($span.element, 'color')).toBe('red') expect(getStyle($span.element, 'width')).toBe(null) expect(getStyle(null, 'color')).toBe(null) - - wrapper.destroy() }) it('select() works', async () => { - const wrapper = mount(App, { - attachTo: document.body - }) - expect(wrapper).toBeDefined() const $btns = wrapper.findAll('div.baz > button') @@ -230,22 +205,15 @@ describe('utils/dom', () => { expect(select('button#button3', wrapper.element)).toBe($btns.at(2).element) expect(select('span.not-here', wrapper.element)).toBe(null) - // Note: It appears that `vue-test-utils` is not detaching previous - // app instances and elements once the test is complete! + // Without root element specified expect(select('button')).not.toBe(null) expect(select('button')).toBe($btns.at(0).element) expect(select('button#button3')).not.toBe(null) expect(select('button#button3')).toBe($btns.at(2).element) expect(select('span.not-here')).toBe(null) - - wrapper.destroy() }) it('selectAll() works', async () => { - const wrapper = mount(App, { - attachTo: document.body - }) - expect(wrapper).toBeDefined() const $btns = wrapper.findAll('div.baz > button') @@ -268,8 +236,6 @@ describe('utils/dom', () => { expect(selectAll('div.baz button', wrapper.element)[2]).toBe($btns.at(2).element) // Without root element specified (assumes document as root) - // Note: It appears that `vue-test-utils` is not detaching previous - // app instances and elements once the test is complete! expect(Array.isArray(selectAll('button'))).toBe(true) expect(selectAll('button')).not.toEqual([]) expect(selectAll('button').length).toBe(3) @@ -285,7 +251,5 @@ describe('utils/dom', () => { expect(selectAll('div.baz button')[0]).toBe($btns.at(0).element) expect(selectAll('div.baz button')[1]).toBe($btns.at(1).element) expect(selectAll('div.baz button')[2]).toBe($btns.at(2).element) - - wrapper.destroy() }) }) From 5c169cc2dad6aac9b65e71e2deed3722bd20e696 Mon Sep 17 00:00:00 2001 From: Charles Rasico <547160+rasicoc@users.noreply.github.com> Date: Wed, 22 Jun 2022 12:17:55 -0500 Subject: [PATCH 2/2] fix(b-tooltip): Updated tooltip to work under shadowDOM Updated tests to get to 100% coverage of changed code + added coverage for isVisible() Tightened the check in the getShadowRootOrRoot() method for document root --- src/utils/dom.js | 5 +-- src/utils/dom.spec.js | 89 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/src/utils/dom.js b/src/utils/dom.js index 4d25b7b7d09..9aed5e9bedb 100644 --- a/src/utils/dom.js +++ b/src/utils/dom.js @@ -84,8 +84,7 @@ export const isActiveElement = el => isElement(el) && el === getActiveElement() // Determine if an HTML element is visible - Faster than CSS check export const isVisible = el => { if (!isElement(el) || !el.parentNode || !isConnectedToDOM(el)) { - // Note this can fail for shadow dom elements since they - // are not a direct descendant of document.body + // Fail for IE11 Shadow DOM. Fixed for all other Browsers return false } if (getStyle(el, 'display') === 'none') { @@ -106,7 +105,7 @@ export const getShadowRootOrRoot = el => { return DOCUMENT.body } const root = el.getRootNode() - if (root.nodeType === 9) { + if (root.nodeName === '#document') { return root.body } return root diff --git a/src/utils/dom.spec.js b/src/utils/dom.spec.js index 54e600560f4..d05b39bb764 100644 --- a/src/utils/dom.spec.js +++ b/src/utils/dom.spec.js @@ -10,6 +10,7 @@ import { isConnectedToDOM, isDisabled, isElement, + isVisible, matches, select, selectAll @@ -36,6 +37,28 @@ describe('utils/dom', () => { wrapper = mount(App, { attachTo: document.body }) + // https://github.com/FezVrasta/popper.js/issues/478#issuecomment-407422016 + // Hack to make Popper not bork out during tests + // Note popper still does not do any positioning calculation in JSDOM though + // So we cannot test actual positioning, just detect when it is open + document.createRange = () => ({ + setStart: () => {}, + setEnd: () => {}, + commonAncestorContainer: { + nodeName: 'BODY', + ownerDocument: document + } + }) + // Mock `getBoundingClientRect()` so that the `isVisible(el)` test returns `true` + // Needed for visibility checks of trigger element, etc + Element.prototype.getBoundingClientRect = jest.fn(() => ({ + width: 24, + height: 24, + top: 0, + left: 0, + bottom: 0, + right: 0 + })) }) afterEach(() => { @@ -61,7 +84,31 @@ describe('utils/dom', () => { expect(isDisabled($btns.at(2).element)).toBe(true) }) - // NOTE: Need to figure out how to test against shadowDOM + it('isVisible() works', async () => { + expect(wrapper).toBeDefined() + const $baz = wrapper.find('div.baz') + expect($baz).toBeDefined() + expect(isVisible($baz.element)).toBe(true) + }) + it('isVisible() is not connected', async () => { + const el = document.createElement('div') + + expect(el).toBeDefined() + // test for element not connected + expect(isVisible(el)).toBe(false) + // test for not an element + expect(isVisible({})).toBe(false) + }) + + it('isVisible() element display none', async () => { + expect(wrapper).toBeDefined() + + const $baz = wrapper.find('div.baz') + $baz.element.style.display = 'none' + expect($baz).toBeDefined() + expect(isVisible($baz.element)).toBe(false) + }) + it('isConnectedToDOM() Regular DOM', async () => { expect(wrapper).toBeDefined() @@ -71,13 +118,51 @@ describe('utils/dom', () => { expect(isConnectedToDOM($barspan.at(0).element)).toBe(true) }) + it('isConnectedToDOM() Shadow DOM', async () => { + const rootEl = document.createElement('div') + const shadow = rootEl.attachShadow({ mode: 'open' }) + const subElement = document.createElement('p') + shadow.appendChild(subElement) + document.body.appendChild(rootEl) + + expect(rootEl).toBeDefined() + expect(shadow).toBeDefined() + expect(isConnectedToDOM(subElement)).toBe(true) + }) + + it('isConnectedToDOM() not connected', async () => { + const el = document.createElement('div') + + expect(el).toBeDefined() + expect(isConnectedToDOM(el)).toBe(false) + }) + + it('getShadowRootOrRoot() Shadow Dom', async () => { + const rootEl = document.createElement('div') + const shadow = rootEl.attachShadow({ mode: 'open' }) + const subElement = document.createElement('p') + shadow.appendChild(subElement) + const testRoot = getShadowRootOrRoot(subElement) + + expect(rootEl).toBeDefined() + expect(shadow).toBeDefined() + expect(testRoot).toBe(shadow) + }) it('getShadowRootOrRoot() Regular DOM', async () => { expect(wrapper).toBeDefined() const $baz = wrapper.find('div.baz') const $documentBody = getShadowRootOrRoot($baz.element) expect($documentBody).toBeDefined() - expect($documentBody.toString()).toBe('[object HTMLBodyElement]') + expect($documentBody.nodeName.toLowerCase()).toBe('body') + }) + + it('getShadowRootOrRoot() getRootNode is Null', async () => { + const el = { + getRootNode: null + } + + expect(getShadowRootOrRoot(el).nodeName.toLowerCase()).toBe('body') }) it('hasClass() works', async () => {