Skip to content

Commit

Permalink
fix(v-b-tooltip, v-b-popover): don't show if no title/content provided (
Browse files Browse the repository at this point in the history
closes #4064) (#4076)
  • Loading branch information
tmorehouse committed Sep 11, 2019
1 parent 9ddd115 commit 0b7de29
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 13 deletions.
4 changes: 2 additions & 2 deletions src/components/popover/helpers/bv-popover-template.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ export const BVPopoverTemplate = /*#__PURE__*/ Vue.extend({
},
[
h('div', { ref: 'arrow', staticClass: 'arrow' }),
isUndefinedOrNull($title)
isUndefinedOrNull($title) || $title === ''
? h()
: h('h3', { staticClass: 'popover-header', domProps: titleDomProps }, [$title]),
isUndefinedOrNull($content)
isUndefinedOrNull($content) || $content === ''
? h()
: h('div', { staticClass: 'popover-body', domProps: contentDomProps }, [$content])
]
Expand Down
17 changes: 14 additions & 3 deletions src/components/tooltip/helpers/bv-tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,14 @@ import {
eventOn,
eventOff
} from '../../../utils/dom'
import { isFunction, isNumber, isPlainObject, isString, isUndefined } from '../../../utils/inspect'
import {
isFunction,
isNumber,
isPlainObject,
isString,
isUndefined,
isUndefinedOrNull
} from '../../../utils/inspect'
import { keys } from '../../../utils/object'
import { warn } from '../../../utils/warn'
import { BvEvent } from '../../../utils/bv-event.class'
Expand Down Expand Up @@ -358,9 +365,13 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
!target ||
!contains(document.body, target) ||
!isVisible(target) ||
this.dropdownOpen()
this.dropdownOpen() ||
((isUndefinedOrNull(this.title) || this.title === '') &&
(isUndefinedOrNull(this.content) || this.content === ''))
) {
// If trigger element isn't in the DOM or is not visible, or is on an open dropdown toggle
// If trigger element isn't in the DOM or is not visible, or
// is on an open dropdown toggle, or has no content, then
// we exit without showing
return
}

Expand Down
3 changes: 2 additions & 1 deletion src/directives/popover/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ to appear.

## Overview

Things to know when using popovers:
Things to know when using the popover directive:

- Popovers rely on the 3rd party library [Popper.js](https://popper.js.org/) for positioning.
- Popovers require BootstrapVue's custom SCSS/CSS for transitions and color variants.
- If both title and content is not provided (or are an empty string), the popover will not show.
- Specify container: 'body' (default) to avoid rendering problems in more complex components (like
input groups, button groups, etc).
- Triggering popovers on hidden elements will not work.
Expand Down
13 changes: 10 additions & 3 deletions src/directives/popover/popover.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@ import looseEqual from '../../utils/loose-equal'
import { concat } from '../../utils/array'
import { getComponentConfig } from '../../utils/config'
import { isBrowser } from '../../utils/env'
import { isFunction, isObject, isString, isUndefined } from '../../utils/inspect'
import {
isFunction,
isObject,
isNumber,
isString,
isUndefined,
isUndefinedOrNull
} from '../../utils/inspect'
import { keys } from '../../utils/object'
import { BVPopover } from '../../components/popover/helpers/bv-popover'

Expand Down Expand Up @@ -58,7 +65,7 @@ const parseBindings = (bindings, vnode) => /* istanbul ignore next: not easy to
}

// Process `bindings.value`
if (isString(bindings.value)) {
if (isString(bindings.value) || isNumber(bindings.value)) {
// Value is popover content (html optionally supported)
config.content = bindings.value
} else if (isFunction(bindings.value)) {
Expand All @@ -80,7 +87,7 @@ const parseBindings = (bindings, vnode) => /* istanbul ignore next: not easy to
if (isUndefined(config.title)) {
// Try attribute
const data = vnode.data || {}
config.title = data.attrs && data.attrs.title ? data.attrs.title : undefined
config.title = data.attrs && !isUndefinedOrNull(data.attrs.title) ? data.attrs.title : undefined
}

// Normalize delay
Expand Down
3 changes: 2 additions & 1 deletion src/directives/tooltip/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ appear.

## Overview

Things to know when using tooltips:
Things to know when using the tooltip directive:

- Tooltips rely on the 3rd party library [Popper.js](https://popper.js.org/) for positioning.
- Tooltips require BootstrapVue's custom SCSS/CSS for transitions and color variants.
- If a title is not provided (or is an empty string), the tooltip will not show.
- Specify container: 'body' (the default) to avoid rendering problems in more complex components
(like input groups, button groups, etc).
- Triggering tooltips on hidden elements will not work.
Expand Down
13 changes: 10 additions & 3 deletions src/directives/tooltip/tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@ import looseEqual from '../../utils/loose-equal'
import { concat } from '../../utils/array'
import { getComponentConfig } from '../../utils/config'
import { isBrowser } from '../../utils/env'
import { isFunction, isObject, isString, isUndefined } from '../../utils/inspect'
import {
isFunction,
isNumber,
isObject,
isString,
isUndefined,
isUndefinedOrNull
} from '../../utils/inspect'
import { keys } from '../../utils/object'
import { BVTooltip } from '../../components/tooltip/helpers/bv-tooltip'

Expand Down Expand Up @@ -58,7 +65,7 @@ const parseBindings = (bindings, vnode) => /* istanbul ignore next: not easy to
}

// Process `bindings.value`
if (isString(bindings.value)) {
if (isString(bindings.value) || isNumber(bindings.value)) {
// Value is tooltip content (HTML optionally supported)
config.title = bindings.value
} else if (isFunction(bindings.value)) {
Expand All @@ -73,7 +80,7 @@ const parseBindings = (bindings, vnode) => /* istanbul ignore next: not easy to
if (isUndefined(config.title)) {
// Try attribute
const data = vnode.data || {}
config.title = data.attrs && data.attrs.title ? data.attrs.title : ''
config.title = data.attrs && !isUndefinedOrNull(data.attrs.title) ? data.attrs.title : undefined
}

// Normalize delay
Expand Down
53 changes: 53 additions & 0 deletions src/directives/tooltip/tooltip.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,59 @@ describe('v-b-tooltip directive', () => {
wrapper.destroy()
})

it('should nowshow tooltip when title is empty', async () => {
jest.useFakeTimers()
const localVue = new CreateLocalVue()

const App = localVue.extend({
directives: {
bTooltip: VBTooltip
},
template: '<button v-b-tooltip.click title="">button</button>'
})

const wrapper = mount(App, {
localVue: localVue,
attachToDocument: true
})

expect(wrapper.isVueInstance()).toBe(true)
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.is('button')).toBe(true)
const $button = wrapper.find('button')

// Should have instance of popover class on it
expect($button.element[BV_TOOLTIP]).toBeDefined()
expect($button.element[BV_TOOLTIP]).toBeInstanceOf(BVTooltip)

expect($button.attributes('aria-describedby')).not.toBeDefined()

// Trigger click
$button.trigger('click')
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($button.attributes('aria-describedby')).not.toBeDefined()

wrapper.destroy()
})

it('variant and customClass should work', async () => {
jest.useFakeTimers()
const localVue = new CreateLocalVue()
Expand Down

0 comments on commit 0b7de29

Please sign in to comment.