From dfb87a7698367e872ffbd8db82c3bb80907a7d2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Fri, 21 Aug 2020 08:39:07 +0200 Subject: [PATCH 1/2] fix(bv-tooltip): hide the tooltip when the title is set to empty --- src/components/tooltip/helpers/bv-tooltip.js | 40 ++++++++++++-------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/components/tooltip/helpers/bv-tooltip.js b/src/components/tooltip/helpers/bv-tooltip.js index 656dbfca221..6f6c9693930 100644 --- a/src/components/tooltip/helpers/bv-tooltip.js +++ b/src/components/tooltip/helpers/bv-tooltip.js @@ -56,6 +56,9 @@ const CONTAINER_SELECTOR = [MODAL_SELECTOR, SIDEBAR_SELECTOR].join(', ') const DROPDOWN_CLASS = 'dropdown' const DROPDOWN_OPEN_SELECTOR = '.dropdown-menu.show' +// Data attribute to temporary store the `title` attribute's value +const DATA_TITLE_ATTR = 'data-original-title' + // Data specific to popper and template // We don't use props, as we need reactivity (we can't pass reactive props) const templateData = { @@ -200,8 +203,18 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ // ensure that the template updates accordingly this.handleTemplateUpdate() }, - disabled(newVal) { - newVal ? this.disable() : this.enable() + title(newValue, oldValue) { + // Make sure to hide the tooltip when the title is set empty + if (newValue !== oldValue && !newValue) { + this.hide() + } + }, + disabled(newValue) { + if (newValue) { + this.disable() + } else { + this.enable() + } } }, created() { @@ -272,10 +285,10 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ } } }) + // If the title has updated, we may need to handle the `title` + // attribute on the trigger target + // We only do this while the template is open if (titleUpdated && this.localShow) { - // If the title has updated, we may need to handle the title - // attribute on the trigger target. We only do this while the - // template is open this.fixTitle() } }, @@ -594,22 +607,19 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ } }, fixTitle() { - // If the target has a title attribute, null it out and - // store on data-title + // If the target has a `title` attribute, remove it and store it on a data attribute const target = this.getTarget() - if (target && getAttr(target, 'title')) { - // We only update title attribute if it has a value - setAttr(target, 'data-original-title', getAttr(target, 'title') || '') + if (target && hasAttr(target, 'title')) { + setAttr(target, DATA_TITLE_ATTR, getAttr(target, 'title') || '') setAttr(target, 'title', '') } }, restoreTitle() { - // If target had a title, restore the title attribute - // and remove the data-title attribute + // If the target had a title, restore it and remove the data attribute const target = this.getTarget() - if (target && hasAttr(target, 'data-original-title')) { - setAttr(target, 'title', getAttr(target, 'data-original-title') || '') - removeAttr(target, 'data-original-title') + if (target && hasAttr(target, DATA_TITLE_ATTR)) { + setAttr(target, 'title', getAttr(target, DATA_TITLE_ATTR) || '') + removeAttr(target, DATA_TITLE_ATTR) } }, // --- BvEvent helpers --- From 1553f1137d7a51e874b7bb746d92a17fd31d1806 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Fri, 21 Aug 2020 08:51:28 +0200 Subject: [PATCH 2/2] Update tooltip.spec.js --- src/components/tooltip/tooltip.spec.js | 66 +++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/src/components/tooltip/tooltip.spec.js b/src/components/tooltip/tooltip.spec.js index 5f8c3e4995e..c6b248042aa 100644 --- a/src/components/tooltip/tooltip.spec.js +++ b/src/components/tooltip/tooltip.spec.js @@ -1119,7 +1119,7 @@ describe('b-tooltip', () => { // Tooltip element should not be in the document expect(document.body.contains(tip)).toBe(false) - expect(document.getElementById(`adb`)).toBe(null) + expect(document.getElementById('adb')).toBe(null) // Try and show element via root event (using ID of trigger button) // Note that this generates a console warning @@ -1157,6 +1157,70 @@ describe('b-tooltip', () => { wrapper.destroy() }) + it('closes when title is set to empty', async () => { + jest.useFakeTimers() + const wrapper = mount(App, { + attachTo: createContainer(), + propsData: { + show: true, + title: 'hello' + } + }) + + expect(wrapper.vm).toBeDefined() + await waitNT(wrapper.vm) + await waitRAF() + await waitNT(wrapper.vm) + await waitRAF() + await waitNT(wrapper.vm) + await waitRAF() + jest.runOnlyPendingTimers() + await waitNT(wrapper.vm) + await waitRAF() + + expect(wrapper.element.tagName).toBe('ARTICLE') + expect(wrapper.attributes('id')).toBeDefined() + expect(wrapper.attributes('id')).toEqual('wrapper') + + // The trigger button + const $button = wrapper.find('button') + expect($button.exists()).toBe(true) + expect($button.attributes('id')).toBeDefined() + expect($button.attributes('id')).toEqual('foo') + expect($button.attributes('title')).not.toBeDefined() + expect($button.attributes('data-original-title')).not.toBeDefined() + expect($button.attributes('aria-describedby')).toBeDefined() + // ID of the tooltip that will be in the body + const adb = $button.attributes('aria-describedby') + + // wrapper + const $tipHolder = wrapper.findComponent(BTooltip) + expect($tipHolder.exists()).toBe(true) + expect($tipHolder.element.nodeType).toEqual(Node.COMMENT_NODE) + + // Find the tooltip element in the document + const tip = document.getElementById(adb) + expect(tip).not.toBe(null) + expect(tip).toBeInstanceOf(HTMLElement) + const $tip = createWrapper(tip) + expect($tip.element.tagName).toBe('DIV') + expect($tip.classes()).toContain('tooltip') + expect($tip.classes()).toContain('b-tooltip') + // Should contain our title prop value + expect($tip.text()).toContain('hello') + + // Change the title prop + await wrapper.setProps({ title: '' }) + await waitRAF() + await waitRAF() + + // Tooltip element should not be in the document + expect(document.body.contains(tip)).toBe(false) + expect(document.getElementById('adb')).toBe(null) + + wrapper.destroy() + }) + it('applies noninteractive class based on noninteractive prop', async () => { jest.useFakeTimers() const wrapper = mount(App, {