Skip to content

fix(bv-tooltip): hide the tooltip when the title is set to empty (closes #5648) #5677

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

Merged
merged 3 commits into from
Aug 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 25 additions & 15 deletions src/components/tooltip/helpers/bv-tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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()
}
},
Expand Down Expand Up @@ -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 ---
Expand Down
66 changes: 65 additions & 1 deletion src/components/tooltip/tooltip.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')

// <b-tooltip> 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, {
Expand Down