Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(tooltips, popovers): fix memory leak (closes #4400) #4401

Merged
merged 10 commits into from
Nov 20, 2019
Merged

fix(tooltips, popovers): fix memory leak (closes #4400) #4401

Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4081c01
fix(v-b-tooltip): memory leaks
jacobmllr95 Nov 19, 2019
3753cda
Merge branch 'dev' into fix-v-b-tooltip-memory-leaks
jacobmllr95 Nov 19, 2019
bbc9dcb
Update bv-tooltip.js
jacobmllr95 Nov 19, 2019
212ab75
Merge branch 'fix-v-b-tooltip-memory-leaks' of https://github.com/boo…
jacobmllr95 Nov 19, 2019
190ac07
Update bv-tooltip.js
jacobmllr95 Nov 19, 2019
01c91bd
Update bv-tooltip.js
jacobmllr95 Nov 19, 2019
0c19aea
Update bv-tooltip.js
jacobmllr95 Nov 19, 2019
1d0712f
Update bv-tooltip.js
jacobmllr95 Nov 19, 2019
e82b394
Update bv-tooltip.js
tmorehouse Nov 19, 2019
a50e93f
Merge branch 'dev' into fix-v-b-tooltip-memory-leaks
tmorehouse Nov 20, 2019
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
126 changes: 52 additions & 74 deletions src/components/tooltip/helpers/bv-tooltip.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import Vue from '../../../utils/vue'
import getScopId from '../../../utils/get-scope-id'
import looseEqual from '../../../utils/loose-equal'
import noop from '../../../utils/noop'
import { arrayIncludes, concat, from as arrayFrom } from '../../../utils/array'
import {
isElement,
Expand Down Expand Up @@ -34,7 +35,6 @@ import {
import { keys } from '../../../utils/object'
import { warn } from '../../../utils/warn'
import { BvEvent } from '../../../utils/bv-event.class'

import { BVTooltipTemplate } from './bv-tooltip-template'

const NAME = 'BVTooltip'
Expand Down Expand Up @@ -203,7 +203,7 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
this.$_hoverState = ''
this.$_visibleInterval = null
this.$_enabled = !this.disabled
this.$_noop = () => {}
this.$_noop = noop.bind(this)

// Destroy ourselves when the parent is destroyed
if (this.$parent) {
Expand Down Expand Up @@ -236,18 +236,14 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
// Remove all handler/listeners
this.unListen()
this.setWhileOpenListeners(false)

// Clear any timeouts/Timers
clearTimeout(this.$_hoverTimeout)
this.$_hoverTimeout = null

// Clear any timeouts/intervals
this.clearHoverTimeout()
this.clearVisibilityInterval()
// Destroy the template
this.destroyTemplate()
this.restoreTitle()
},
methods: {
//
// Methods for creating and destroying the template
//
// --- Methods for creating and destroying the template ---
getTemplate() {
// Overridden by BVPopover
return BVTooltipTemplate
Expand All @@ -273,7 +269,6 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
},
createTemplateAndShow() {
// Creates the template instance and show it
// this.destroyTemplate()
const container = this.getContainer()
const Template = this.getTemplate()
const $tip = (this.$_tip = new Template({
Expand Down Expand Up @@ -323,19 +318,24 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
// then emit the `hidden` event once it is fully hidden
// The `hook:destroyed` will also be called (safety measure)
this.$_tip && this.$_tip.hide()
// Clear out any stragging active triggers
this.clearActiveTriggers()
// Reset the hover state
this.$_hoverState = ''
},
// Destroy the template instance and reset state
destroyTemplate() {
// Destroy the template instance and reset state
this.setWhileOpenListeners(false)
clearTimeout(this.$_hoverTimeout)
this.$_hoverTimout = null
this.clearHoverTimeout()
this.$_hoverState = ''
this.clearActiveTriggers()
this.localPlacementTarget = null
try {
this.$_tip && this.$_tip.$destroy()
} catch {}
this.$_tip = null
this.removeAriaDescribedby()
this.restoreTitle()
this.localShow = false
},
getTemplateElement() {
Expand All @@ -355,13 +355,10 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
})
}
},
//
// Show and Hide handlers
//
// --- Show/Hide handlers ---
// Show the tooltip
show() {
// Show the tooltip
const target = this.getTarget()

if (
!target ||
!contains(document.body, target) ||
Expand All @@ -375,38 +372,29 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
// we exit without showing
return
}

// If tip already exists, exit early
if (this.$_tip || this.localShow) {
// If tip already exists, exit early
/* istanbul ignore next */
return
}

// In the process of showing
this.localShow = true

// Create a cancelable BvEvent
const showEvt = this.buildEvent('show', { cancelable: true })
this.emitEvent(showEvt)
// Don't show if event cancelled
/* istanbul ignore next: ignore for now */
if (showEvt.defaultPrevented) {
// Don't show if event cancelled
// Destroy the template (if for some reason it was created)
/* istanbul ignore next */
this.destroyTemplate()
// Clear the localShow flag
/* istanbul ignore next */
this.localShow = false
/* istanbul ignore next */
return
}

// Fix the title attribute on target
this.fixTitle()

// Set aria-describedby on target
this.addAriaDescribedby()

// Create and show the tooltip
this.createTemplateAndShow()
},
Expand All @@ -433,11 +421,6 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({

// Tell the template to hide
this.hideTemplate()
// TODO: The following could be added to `hideTemplate()`
// Clear out any stragging active triggers
this.clearActiveTriggers()
// Reset the hover state
this.$_hoverState = ''
},
forceHide() {
// Forcefully hides/destroys the template, regardless of any active triggers
Expand All @@ -450,8 +433,7 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
// This is also done in the template `hide` evt handler
this.setWhileOpenListeners(false)
// Clear any hover enter/leave event
clearTimeout(this.hoverTimeout)
this.$_hoverTimeout = null
this.clearHoverTimeout()
this.$_hoverState = ''
this.clearActiveTriggers()
// Disable the fade animation on the template
Expand All @@ -464,49 +446,42 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
enable() {
this.$_enabled = true
// Create a non-cancelable BvEvent
this.emitEvent(this.buildEvent('enabled', {}))
this.emitEvent(this.buildEvent('enabled'))
},
disable() {
this.$_enabled = false
// Create a non-cancelable BvEvent
this.emitEvent(this.buildEvent('disabled', {}))
this.emitEvent(this.buildEvent('disabled'))
},
//
// Handlers for template events
//
// --- Handlers for template events ---
// When template is inserted into DOM, but not yet shown
onTemplateShow() {
// When template is inserted into DOM, but not yet shown
// Enable while open listeners/watchers
this.setWhileOpenListeners(true)
},
// When template show transition completes
onTemplateShown() {
// When template show transition completes
const prevHoverState = this.$_hoverState
this.$_hoverState = ''
if (prevHoverState === 'out') {
this.leave(null)
}
// Emit a non-cancelable BvEvent 'shown'
this.emitEvent(this.buildEvent('shown', {}))
this.emitEvent(this.buildEvent('shown'))
},
// When template is starting to hide
onTemplateHide() {
// When template is starting to hide
// Disable while open listeners/watchers
this.setWhileOpenListeners(false)
},
// When template has completed closing (just before it self destructs)
onTemplateHidden() {
// When template has completed closing (just before it self destructs)
// TODO:
// The next two lines could be moved into `destroyTemplate()`
this.removeAriaDescribedby()
this.restoreTitle()
// Destroy the template
this.destroyTemplate()
// Emit a non-cancelable BvEvent 'shown'
this.emitEvent(this.buildEvent('hidden', {}))
},
//
// Utility methods
//
// --- Utility methods ---
getTarget() {
// Handle case where target may be a component ref
let target = this.target ? this.target.$el || this.target : null
Expand Down Expand Up @@ -566,6 +541,18 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
const target = this.getTarget()
return this.isDropdown() && target && select(DROPDOWN_OPEN_SELECTOR, target)
},
clearHoverTimeout() {
if (this.$_hoverTimeout) {
clearTimeout(this.$_hoverTimeout)
this.$_hoverTimeout = null
}
},
clearVisibilityInterval() {
if (this.$_visibleInterval) {
clearInterval(this.$_visibleInterval)
this.$_visibleInterval = null
}
},
clearActiveTriggers() {
for (const trigger in this.activeTrigger) {
this.activeTrigger[trigger] = false
Expand Down Expand Up @@ -619,9 +606,7 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
removeAttr(target, 'data-original-title')
}
},
//
// BvEvent helpers
//
// --- BvEvent helpers ---
buildEvent(type, opts = {}) {
// Defaults to a non-cancellable event
return new BvEvent(type, {
Expand All @@ -644,20 +629,16 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
}
this.$emit(evtName, bvEvt)
},
//
// Event handler setup methods
//
// --- Event handler setup methods ---
listen() {
// Enable trigger event handlers
const el = this.getTarget()
if (!el) {
/* istanbul ignore next */
return
}

// Listen for global show/hide events
this.setRootListener(true)

// Set up our listeners on the target trigger element
this.computedTriggers.forEach(trigger => {
if (trigger === 'click') {
Expand Down Expand Up @@ -712,14 +693,13 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
// On-touch start listeners
this.setOnTouchStartListener(on)
},
// Handler for periodic visibility check
visibleCheck(on) {
// Handler for periodic visibility check
clearInterval(this.$_visibleInterval)
this.$_visibleInterval = null
this.clearVisibilityInterval()
const target = this.getTarget()
const tip = this.getTemplateElement()
if (on) {
this.visibleInterval = setInterval(() => {
this.$_visibleInterval = setInterval(() => {
if (tip && this.localShow && (!target.parentNode || !isVisible(target))) {
// Target element is no longer visible or not in DOM, so force-hide the tooltip
this.forceHide()
Expand Down Expand Up @@ -762,9 +742,7 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
target.__vue__[on ? '$on' : '$off']('shown', this.forceHide)
}
},
//
// Event handlers
//
// --- Event handlers ---
handleEvent(evt) {
// General trigger event handler
// target is the trigger element
Expand Down Expand Up @@ -884,14 +862,14 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
this.$_hoverState = 'in'
return
}
clearTimeout(this.hoverTimeout)
this.clearHoverTimeout()
this.$_hoverState = 'in'
if (!this.computedDelay.show) {
this.show()
} else {
// Hide any title attribute while enter delay is active
this.fixTitle()
this.hoverTimeout = setTimeout(() => {
this.$_hoverTimeout = setTimeout(() => {
/* istanbul ignore else */
if (this.$_hoverState === 'in') {
this.show()
Expand All @@ -917,12 +895,12 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({
if (this.isWithActiveTrigger) {
return
}
clearTimeout(this.hoverTimeout)
this.clearHoverTimeout()
this.$_hoverState = 'out'
if (!this.computedDelay.hide) {
this.hide()
} else {
this.$hoverTimeout = setTimeout(() => {
this.$_hoverTimeout = setTimeout(() => {
if (this.$_hoverState === 'out') {
this.hide()
}
Expand Down