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.

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

fix(b-dropdown): handle issue with touch devices on MacOS using Safari/Firefox (Fixes #4328, #4344) #4329

Merged
merged 42 commits into from Nov 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
c37c7a2
fix(b-dropdown): handle issue with touch devices on MacOS using Safar…
tmorehouse Oct 31, 2019
aa1025b
Update dropdown.js
tmorehouse Oct 31, 2019
58980dd
Update dropdown.js
tmorehouse Oct 31, 2019
184bb11
Merge branch 'dev' into issue/4328
tmorehouse Oct 31, 2019
e482b1c
Merge branch 'dev' into issue/4328
tmorehouse Oct 31, 2019
38808cc
Merge branch 'dev' into issue/4328
jacobmllr95 Oct 31, 2019
853751d
Merge branch 'dev' into issue/4328
jacobmllr95 Nov 1, 2019
a9a395b
Update dropdown.js
jacobmllr95 Nov 1, 2019
9b15e3b
Update dropdown.spec.js
jacobmllr95 Nov 1, 2019
ce2c784
Update dropdown.spec.js
jacobmllr95 Nov 1, 2019
c4cfe2d
Update dropdown.spec.js
jacobmllr95 Nov 1, 2019
64b1e46
Update dropdown.js
jacobmllr95 Nov 1, 2019
7d88166
Merge branch 'dev' into issue/4328
tmorehouse Nov 1, 2019
47be13c
Update dropdown.js
jacobmllr95 Nov 1, 2019
3c09ba5
Update dropdown.js
tmorehouse Nov 1, 2019
e70c610
Update dropdown.js
jacobmllr95 Nov 1, 2019
23cbc64
Merge branch 'issue/4328' of https://github.com/bootstrap-vue/bootstr…
jacobmllr95 Nov 1, 2019
a2f3ec5
Update dropdown.js
jacobmllr95 Nov 1, 2019
8a648d3
Update dropdown.js
jacobmllr95 Nov 1, 2019
2c9a658
add in iOS event delegation fix
tmorehouse Nov 1, 2019
5cebdfd
Update dropdown.js
tmorehouse Nov 1, 2019
0165a7f
Update dropdown.js
tmorehouse Nov 1, 2019
7c74da7
Update dropdown.js
tmorehouse Nov 1, 2019
7855ae6
Update dropdown.js
tmorehouse Nov 1, 2019
c98c669
make clickout listener passive (as it should never `preventDefault()`)
tmorehouse Nov 1, 2019
3fd2f02
Update dropdown.js
jacobmllr95 Nov 1, 2019
4722130
Update dropdown.js
jacobmllr95 Nov 3, 2019
7505f25
Update click-out.js
jacobmllr95 Nov 3, 2019
ec093e4
Update focus-in.js
jacobmllr95 Nov 3, 2019
8137b08
Update dropdown.js
jacobmllr95 Nov 3, 2019
b05e0a2
Update dropdown.js
jacobmllr95 Nov 3, 2019
f693744
Merge branch 'dev' into issue/4328
jacobmllr95 Nov 4, 2019
0b284b1
Update dropdown.js
jacobmllr95 Nov 4, 2019
c758e18
improve coverage
tmorehouse Nov 4, 2019
0cc25fe
Update dropdown.js
jacobmllr95 Nov 4, 2019
70d061a
Update dropdown.js
jacobmllr95 Nov 4, 2019
83d126c
Update dropdown.js
jacobmllr95 Nov 4, 2019
f15281d
Update dropdown.js
jacobmllr95 Nov 4, 2019
fa8d033
Update package.json
tmorehouse Nov 5, 2019
3ccc255
Update package.json
tmorehouse Nov 5, 2019
9e899f0
Merge branch 'dev' into issue/4328
jacobmllr95 Nov 8, 2019
1580cd5
Merge branch 'dev' into issue/4328
tmorehouse Nov 8, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 13 additions & 16 deletions src/components/dropdown/dropdown.spec.js
Expand Up @@ -417,8 +417,9 @@ describe('dropdown', () => {
const localVue = new CreateLocalVue()
const App = localVue.extend({
render(h) {
return h('div', {}, [
h(BDropdown, { props: { id: 'test' } }, [h(BDropdownItem, {}, 'item')])
return h('div', { attrs: { id: 'container' } }, [
h(BDropdown, { props: { id: 'test' } }, [h(BDropdownItem, {}, 'item')]),
h('input', { attrs: { id: 'input' } })
])
}
})
Expand All @@ -434,10 +435,12 @@ describe('dropdown', () => {
expect(wrapper.findAll('.dropdown-menu').length).toBe(1)
expect(wrapper.findAll('.dropdown-menu .dropdown-item').length).toBe(1)

const $container = wrapper.find('#container')
const $dropdown = wrapper.find('.dropdown')
const $toggle = wrapper.find('.dropdown-toggle')
const $menu = wrapper.find('.dropdown-menu')
const $item = wrapper.find('.dropdown-item')
const $input = wrapper.find('#input')

expect($dropdown.isVueInstance()).toBe(true)

Expand Down Expand Up @@ -480,21 +483,21 @@ describe('dropdown', () => {
expect($toggle.attributes('aria-expanded')).toEqual('false')
expect($dropdown.classes()).not.toContain('show')

// Open menu via .show() method
// Open menu via ´.show()´ method
$dropdown.vm.show()
await waitNT(wrapper.vm)
await waitRAF()
expect($toggle.attributes('aria-expanded')).toEqual('true')
expect($dropdown.classes()).toContain('show')

// Close menu via .hide() method
// Close menu via ´.hide()´ method
$dropdown.vm.hide()
await waitNT(wrapper.vm)
await waitRAF()
expect($toggle.attributes('aria-expanded')).toEqual('false')
expect($dropdown.classes()).not.toContain('show')

// Open menu via .show() method again
// Open menu via ´.show()´ method again
$dropdown.vm.show()
await waitNT(wrapper.vm)
await waitRAF()
Expand All @@ -503,10 +506,7 @@ describe('dropdown', () => {
expect(document.activeElement).toBe($menu.element)

// Close menu by moving focus away from menu
// which triggers a focusout event on menu
$menu.trigger('focusout', {
relatedTarget: document.body
})
$input.trigger('focusin')
await waitNT(wrapper.vm)
await waitRAF()
expect($dropdown.classes()).not.toContain('show')
Expand All @@ -520,17 +520,14 @@ describe('dropdown', () => {
expect($toggle.attributes('aria-expanded')).toEqual('true')
expect(document.activeElement).toBe($menu.element)

// Close menu by moving focus away from menu
// which triggers a focusout event on menu
$menu.trigger('focusout', {
relatedTarget: document.body
})
// Close menu by clicking outside
$container.trigger('click')
await waitNT(wrapper.vm)
await waitRAF()
expect($dropdown.classes()).not.toContain('show')
expect($toggle.attributes('aria-expanded')).toEqual('false')

// Open menu via .show() method again
// Open menu via ´.show()´ method again
$dropdown.vm.show()
await waitNT(wrapper.vm)
await waitRAF()
Expand All @@ -544,7 +541,7 @@ describe('dropdown', () => {
expect($dropdown.classes()).not.toContain('show')
expect($toggle.attributes('aria-expanded')).toEqual('false')

// Open menu via .show() method again
// Open menu via ´.show()´ method again
$dropdown.vm.show()
await waitNT(wrapper.vm)
await waitRAF()
Expand Down
3 changes: 2 additions & 1 deletion src/components/dropdown/package.json
Expand Up @@ -41,7 +41,7 @@
},
{
"prop": "offset",
"description": "Specify the number of pixes to shift the menu by. Negative values supported"
"description": "Specify the number of pixels to shift the menu by. Negative values supported"
},
{
"prop": "lazy",
Expand Down Expand Up @@ -69,6 +69,7 @@
},
{
"prop": "block",
"version": "2.1.0",
"description": "Renders a 100% width toggle button (expands to the width of it's parent container)"
},
{
Expand Down
10 changes: 6 additions & 4 deletions src/mixins/click-out.js
@@ -1,5 +1,7 @@
import { contains, eventOff, eventOn } from '../utils/dom'

const eventOptions = { passive: true, capture: false }

// @vue/component
export default {
data() {
Expand All @@ -10,9 +12,9 @@ export default {
watch: {
listenForClickOut(newValue, oldValue) {
if (newValue !== oldValue) {
eventOff(this.clickOutElement, this.clickOutEventName, this._clickOutHandler, false)
eventOff(this.clickOutElement, this.clickOutEventName, this._clickOutHandler, eventOptions)
if (newValue) {
eventOn(this.clickOutElement, this.clickOutEventName, this._clickOutHandler, false)
eventOn(this.clickOutElement, this.clickOutEventName, this._clickOutHandler, eventOptions)
}
}
}
Expand All @@ -30,11 +32,11 @@ export default {
this.clickOutEventName = 'ontouchstart' in document.documentElement ? 'touchstart' : 'click'
}
if (this.listenForClickOut) {
eventOn(this.clickOutElement, this.clickOutEventName, this._clickOutHandler, false)
eventOn(this.clickOutElement, this.clickOutEventName, this._clickOutHandler, eventOptions)
}
},
beforeDestroy() /* istanbul ignore next */ {
eventOff(this.clickOutElement, this.clickOutEventName, this._clickOutHandler, false)
eventOff(this.clickOutElement, this.clickOutEventName, this._clickOutHandler, eventOptions)
},
methods: {
isClickOut(evt) {
Expand Down
80 changes: 46 additions & 34 deletions src/mixins/dropdown.js
Expand Up @@ -2,8 +2,11 @@ import Popper from 'popper.js'
import KeyCodes from '../utils/key-codes'
import warn from '../utils/warn'
import { BvEvent } from '../utils/bv-event.class'
import { closest, contains, isVisible, requestAF, selectAll, eventOn, eventOff } from '../utils/dom'
import { closest, contains, isVisible, requestAF, selectAll } from '../utils/dom'
import { hasTouchSupport } from '../utils/env'
import { isNull } from '../utils/inspect'
import clickOutMixin from './click-out'
import focusInMixin from './focus-in'
import idMixin from './id'

// Return an array of visible items
Expand All @@ -15,7 +18,7 @@ const ROOT_DROPDOWN_SHOWN = `${ROOT_DROPDOWN_PREFIX}shown`
const ROOT_DROPDOWN_HIDDEN = `${ROOT_DROPDOWN_PREFIX}hidden`

// Delay when loosing focus before closing menu (in ms)
const FOCUSOUT_DELAY = 100
const FOCUSOUT_DELAY = hasTouchSupport ? 450 : 150

// Dropdown item CSS selectors
const Selector = {
Expand Down Expand Up @@ -47,7 +50,7 @@ const AttachmentMap = {

// @vue/component
export default {
mixins: [idMixin],
mixins: [idMixin, clickOutMixin, focusInMixin],
provide() {
return {
bvDropdown: this
Expand Down Expand Up @@ -171,18 +174,21 @@ export default {
},
created() {
// Create non-reactive property
this._popper = null
this.$_popper = null
this.$_hideTimeout = null
this.$_noop = () => {}
},
deactivated() /* istanbul ignore next: not easy to test */ {
// In case we are inside a `<keep-alive>`
this.visible = false
this.whileOpenListen(false)
this.removePopper()
this.destroyPopper()
},
beforeDestroy() {
this.visible = false
this.whileOpenListen(false)
this.removePopper()
this.destroyPopper()
this.clearHideTimeout()
},
methods: {
// Event emitter
Expand Down Expand Up @@ -235,18 +241,25 @@ export default {
this.whileOpenListen(false)
this.$root.$emit(ROOT_DROPDOWN_HIDDEN, this)
this.$emit('hidden')
this.removePopper()
this.destroyPopper()
},
createPopper(element) {
this.removePopper()
this._popper = new Popper(element, this.$refs.menu, this.getPopperConfig())
this.destroyPopper()
this.$_popper = new Popper(element, this.$refs.menu, this.getPopperConfig())
},
removePopper() {
if (this._popper) {
destroyPopper() {
if (this.$_popper) {
// Ensure popper event listeners are removed cleanly
this._popper.destroy()
this.$_popper.destroy()
}
this.$_popper = null
},
clearHideTimeout() {
/* istanbul ignore next */
if (this.$_hideTimeout) {
clearTimeout(this.$_hideTimeout)
this.$_hideTimeout = null
}
this._popper = null
},
getPopperConfig() {
let placement = AttachmentMap.BOTTOM
Expand All @@ -271,17 +284,15 @@ export default {
}
return { ...popperConfig, ...(this.popperOpts || {}) }
},
// Turn listeners on/off while open
whileOpenListen(isOpen) {
// turn listeners on/off while open
if (isOpen) {
// If another dropdown is opened
this.$root.$on(ROOT_DROPDOWN_SHOWN, this.rootCloseListener)
// Hide the menu when focus moves out
eventOn(this.$el, 'focusout', this.onFocusOut, { passive: true })
} else {
this.$root.$off(ROOT_DROPDOWN_SHOWN, this.rootCloseListener)
eventOff(this.$el, 'focusout', this.onFocusOut, { passive: true })
}
// Hide the dropdown when clicked outside
this.listenForClickOut = isOpen
// Hide the dropdown when it loses focus
this.listenForFocusIn = isOpen
// Hide the dropdown when another dropdown is opened
const method = isOpen ? '$on' : '$off'
this.$root[method](ROOT_DROPDOWN_SHOWN, this.rootCloseListener)
},
rootCloseListener(vm) {
if (vm !== this) {
Expand Down Expand Up @@ -375,27 +386,28 @@ export default {
this.$once('hidden', this.focusToggler)
}
},
// Dropdown wrapper focusOut handler
onFocusOut(evt) {
// `relatedTarget` is the element gaining focus
const relatedTarget = evt.relatedTarget
// If focus moves outside the menu or toggler, then close menu
if (
this.visible &&
!contains(this.$refs.menu, relatedTarget) &&
!contains(this.toggler, relatedTarget)
) {
// Document click out listener
clickOutHandler(evt) {
const target = evt.target
if (this.visible && !contains(this.$refs.menu, target) && !contains(this.toggler, target)) {
const doHide = () => {
this.visible = false
return null
}
// When we are in a navbar (which has been responsively stacked), we
// delay the dropdown's closing so that the next element has a chance
// to have it's click handler fired (in case it's position moves on
// the screen do to a navbar menu above it collapsing)
// https://github.com/bootstrap-vue/bootstrap-vue/issues/4113
this.inNavbar ? setTimeout(doHide, FOCUSOUT_DELAY) : doHide()
this.clearHideTimeout()
this.$_hideTimeout = this.inNavbar ? setTimeout(doHide, FOCUSOUT_DELAY) : doHide()
}
},
// Document focusin listener
focusInHandler(evt) {
// Shared logic with click-out handler
this.clickOutHandler(evt)
},
// Keyboard nav
focusNext(evt, up) {
// Ignore key up/down on form elements
Expand Down
10 changes: 6 additions & 4 deletions src/mixins/focus-in.js
@@ -1,5 +1,7 @@
import { eventOff, eventOn } from '../utils/dom'

const eventOptions = { passive: true, capture: false }

// @vue/component
export default {
data() {
Expand All @@ -10,9 +12,9 @@ export default {
watch: {
listenForFocusIn(newValue, oldValue) {
if (newValue !== oldValue) {
eventOff(this.focusInElement, 'focusin', this._focusInHandler, false)
eventOff(this.focusInElement, 'focusin', this._focusInHandler, eventOptions)
if (newValue) {
eventOn(this.focusInElement, 'focusin', this._focusInHandler, false)
eventOn(this.focusInElement, 'focusin', this._focusInHandler, eventOptions)
}
}
}
Expand All @@ -26,11 +28,11 @@ export default {
this.focusInElement = document
}
if (this.listenForFocusIn) {
eventOn(this.focusInElement, 'focusin', this._focusInHandler, false)
eventOn(this.focusInElement, 'focusin', this._focusInHandler, eventOptions)
}
},
beforeDestroy() /* istanbul ignore next */ {
eventOff(this.focusInElement, 'focusin', this._focusInHandler, false)
eventOff(this.focusInElement, 'focusin', this._focusInHandler, eventOptions)
},
methods: {
_focusInHandler(evt) {
Expand Down