Skip to content

Commit 4705091

Browse files
jacobmllr95tmorehouse
jacobmllr95
authored and
tmorehouse
committed
chore: directives code cleanup (#2927)
1 parent 2e2542f commit 4705091

File tree

11 files changed

+552
-182
lines changed

11 files changed

+552
-182
lines changed

‎docs/nuxt.config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ module.exports = {
117117
.readdirSync(`${root}/${dir}`)
118118
.filter(c => c !== 'index.js' && c[0] !== '_')
119119
.filter(c => excludeDirs.indexOf(c) === -1)
120+
.filter(c => !/\.s?css$/.test(c))
120121
.map(page => `/docs/${dir}/${page}`)
121122

122123
return []

‎src/directives/modal/modal.js

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,38 @@
1-
import { bindTargets, unbindTargets } from '../../utils/target'
21
import { setAttr, removeAttr } from '../../utils/dom'
2+
import { bindTargets, unbindTargets } from '../../utils/target'
33

4+
// Target listen types
45
const listenTypes = { click: true }
56

7+
// Emitted show event for modal
8+
const EVENT_SHOW = 'bv::show::modal'
9+
10+
const setRole = (el, binding, vnode) => {
11+
if (el.tagName !== 'BUTTON') {
12+
setAttr(el, 'role', 'button')
13+
}
14+
}
15+
16+
/*
17+
* Export our directive
18+
*/
619
export default {
720
// eslint-disable-next-line no-shadow-restricted-names
821
bind(el, binding, vnode) {
922
bindTargets(vnode, binding, listenTypes, ({ targets, vnode }) => {
1023
targets.forEach(target => {
11-
vnode.context.$root.$emit('bv::show::modal', target, vnode.elm)
24+
vnode.context.$root.$emit(EVENT_SHOW, target, vnode.elm)
1225
})
1326
})
14-
if (el.tagName !== 'BUTTON') {
15-
// If element is not a button, we add `role="button"` for accessibility
16-
setAttr(el, 'role', 'button')
17-
}
27+
// If element is not a button, we add `role="button"` for accessibility
28+
setRole(el, binding, vnode)
1829
},
30+
updated: setRole,
31+
componentUpdated: setRole,
1932
unbind(el, binding, vnode) {
2033
unbindTargets(vnode, binding, listenTypes)
34+
// If element is not a button, we add `role="button"` for accessibility
2135
if (el.tagName !== 'BUTTON') {
22-
// If element is not a button, we add `role="button"` for accessibility
2336
removeAttr(el, 'role', 'button')
2437
}
2538
}

‎src/directives/modal/modal.spec.js

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import modalDirective from './modal'
2+
import { mount, createLocalVue as CreateLocalVue } from '@vue/test-utils'
3+
4+
const EVENT_SHOW = 'bv::show::modal'
5+
6+
describe('v-b-modal directive', () => {
7+
it('works on buttons', async () => {
8+
const localVue = new CreateLocalVue()
9+
const spy = jest.fn()
10+
11+
const App = localVue.extend({
12+
directives: {
13+
bModal: modalDirective
14+
},
15+
data() {
16+
return {}
17+
},
18+
mounted() {
19+
this.$root.$on(EVENT_SHOW, spy)
20+
},
21+
beforeDestroy() {
22+
this.$root.$off(EVENT_SHOW, spy)
23+
},
24+
template: '<button v-b-modal.test>button</button>'
25+
})
26+
const wrapper = mount(App, {
27+
localVue: localVue
28+
})
29+
30+
expect(wrapper.isVueInstance()).toBe(true)
31+
expect(wrapper.is('button')).toBe(true)
32+
expect(spy).not.toHaveBeenCalled()
33+
34+
const $button = wrapper.find('button')
35+
$button.trigger('click')
36+
expect(spy).toHaveBeenCalledTimes(1)
37+
expect(spy).toBeCalledWith('test', $button.element)
38+
39+
wrapper.destroy()
40+
})
41+
42+
it('works on non-buttons', async () => {
43+
const localVue = new CreateLocalVue()
44+
const spy = jest.fn()
45+
46+
const App = localVue.extend({
47+
directives: {
48+
bModal: modalDirective
49+
},
50+
data() {
51+
return {
52+
text: 'span'
53+
}
54+
},
55+
mounted() {
56+
this.$root.$on(EVENT_SHOW, spy)
57+
},
58+
beforeDestroy() {
59+
this.$root.$off(EVENT_SHOW, spy)
60+
},
61+
template: '<span tabindex="0" v-b-modal.test>{{ text }}</span>'
62+
})
63+
const wrapper = mount(App, {
64+
localVue: localVue
65+
})
66+
67+
expect(wrapper.isVueInstance()).toBe(true)
68+
expect(wrapper.is('span')).toBe(true)
69+
expect(spy).not.toHaveBeenCalled()
70+
expect(wrapper.find('span').attributes('role')).toBe('button')
71+
expect(wrapper.find('span').text()).toBe('span')
72+
73+
const $span = wrapper.find('span')
74+
$span.trigger('click')
75+
expect(spy).toHaveBeenCalledTimes(1)
76+
expect(spy).toBeCalledWith('test', $span.element)
77+
expect(wrapper.find('span').attributes('role')).toBe('button')
78+
79+
// Test updating component. should maintain role attribute
80+
wrapper.setData({
81+
text: 'foobar'
82+
})
83+
expect(wrapper.find('span').text()).toBe('foobar')
84+
expect(wrapper.find('span').attributes('role')).toBe('button')
85+
86+
wrapper.destroy()
87+
})
88+
})

‎src/directives/popover/popover.js

Lines changed: 39 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
import Popper from 'popper.js'
22
import PopOver from '../../utils/popover.class'
3+
import { inBrowser } from '../../utils/env'
34
import { keys } from '../../utils/object'
45
import warn from '../../utils/warn'
56

6-
const inBrowser = typeof window !== 'undefined' && typeof document !== 'undefined'
7-
87
// Key which we use to store tooltip object on element
9-
const BVPO = '__BV_PopOver__'
8+
const BV_POPOVER = '__BV_PopOver__'
109

1110
// Valid event triggers
1211
const validTriggers = {
@@ -17,9 +16,9 @@ const validTriggers = {
1716
}
1817

1918
// Build a PopOver config based on bindings (if any)
20-
// Arguments and modifiers take precedence over pased value config object
19+
// Arguments and modifiers take precedence over passed value config object
2120
/* istanbul ignore next: not easy to test */
22-
function parseBindings(bindings) {
21+
const parseBindings = bindings => /* istanbul ignore next: not easy to test */ {
2322
// We start out with a blank config
2423
let config = {}
2524

@@ -35,9 +34,10 @@ function parseBindings(bindings) {
3534
config = { ...config, ...bindings.value }
3635
}
3736

38-
// If Argument, assume element ID of container element
37+
// If argument, assume element ID of container element
3938
if (bindings.arg) {
40-
// Element ID specified as arg. We must prepend '#' to become a CSS selector
39+
// Element ID specified as arg
40+
// We must prepend '#' to become a CSS selector
4141
config.container = `#${bindings.arg}`
4242
}
4343

@@ -55,35 +55,36 @@ function parseBindings(bindings) {
5555
// placement of popover
5656
config.placement = mod
5757
} else if (/^(window|viewport)$/.test(mod)) {
58-
// bounday of popover
58+
// Boundary of popover
5959
config.boundary = mod
6060
} else if (/^d\d+$/.test(mod)) {
61-
// delay value
61+
// Delay value
6262
const delay = parseInt(mod.slice(1), 10) || 0
6363
if (delay) {
6464
config.delay = delay
6565
}
6666
} else if (/^o-?\d+$/.test(mod)) {
67-
// offset value (negative allowed)
67+
// Offset value (negative allowed)
6868
const offset = parseInt(mod.slice(1), 10) || 0
6969
if (offset) {
7070
config.offset = offset
7171
}
7272
}
7373
})
7474

75-
// Special handling of event trigger modifiers Trigger is a space separated list
75+
// Special handling of event trigger modifiers trigger is
76+
// a space separated list
7677
const selectedTriggers = {}
7778

78-
// parse current config object trigger
79+
// Parse current config object trigger
7980
let triggers = typeof config.trigger === 'string' ? config.trigger.trim().split(/\s+/) : []
8081
triggers.forEach(trigger => {
8182
if (validTriggers[trigger]) {
8283
selectedTriggers[trigger] = true
8384
}
8485
})
8586

86-
// Parse Modifiers for triggers
87+
// Parse modifiers for triggers
8788
keys(validTriggers).forEach(trigger => {
8889
if (bindings.modifiers[trigger]) {
8990
selectedTriggers[trigger] = true
@@ -97,70 +98,64 @@ function parseBindings(bindings) {
9798
config.trigger = 'focus'
9899
}
99100
if (!config.trigger) {
100-
// remove trigger config
101+
// Remove trigger config
101102
delete config.trigger
102103
}
103104

104105
return config
105106
}
106107

107-
//
108-
// Add or Update popover on our element
109-
//
110-
/* istanbul ignore next: not easy to test */
111-
function applyBVPO(el, bindings, vnode) {
108+
// Add or update PopOver on our element
109+
const applyPopover = (el, bindings, vnode) => {
112110
if (!inBrowser) {
111+
/* istanbul ignore next */
113112
return
114113
}
114+
// Popper is required for PopOvers to work
115115
if (!Popper) {
116-
// Popper is required for tooltips to work
117-
warn('v-b-popover: Popper.js is required for popovers to work')
116+
/* istanbul ignore next */
117+
warn('v-b-popover: Popper.js is required for PopOvers to work')
118+
/* istanbul ignore next */
118119
return
119120
}
120-
if (el[BVPO]) {
121-
el[BVPO].updateConfig(parseBindings(bindings))
121+
const config = parseBindings(bindings)
122+
if (el[BV_POPOVER]) {
123+
el[BV_POPOVER].updateConfig(config)
122124
} else {
123-
el[BVPO] = new PopOver(el, parseBindings(bindings), vnode.context.$root)
125+
el[BV_POPOVER] = new PopOver(el, config, vnode.context.$root)
124126
}
125127
}
126128

127-
//
128-
// Remove popover on our element
129-
//
130-
/* istanbul ignore next */
131-
function removeBVPO(el) {
132-
if (!inBrowser) {
133-
return
134-
}
135-
if (el[BVPO]) {
136-
el[BVPO].destroy()
137-
el[BVPO] = null
138-
delete el[BVPO]
129+
// Remove PopOver on our element
130+
const removePopover = el => {
131+
if (el[BV_POPOVER]) {
132+
el[BV_POPOVER].destroy()
133+
el[BV_POPOVER] = null
134+
delete el[BV_POPOVER]
139135
}
140136
}
141137

142138
/*
143139
* Export our directive
144140
*/
145-
/* istanbul ignore next: not easy to test */
146141
export default {
147142
bind(el, bindings, vnode) {
148-
applyBVPO(el, bindings, vnode)
143+
applyPopover(el, bindings, vnode)
149144
},
150145
inserted(el, bindings, vnode) {
151-
applyBVPO(el, bindings, vnode)
146+
applyPopover(el, bindings, vnode)
152147
},
153-
update(el, bindings, vnode) {
148+
update(el, bindings, vnode) /* istanbul ignore next: not easy to test */ {
154149
if (bindings.value !== bindings.oldValue) {
155-
applyBVPO(el, bindings, vnode)
150+
applyPopover(el, bindings, vnode)
156151
}
157152
},
158-
componentUpdated(el, bindings, vnode) {
153+
componentUpdated(el, bindings, vnode) /* istanbul ignore next: not easy to test */ {
159154
if (bindings.value !== bindings.oldValue) {
160-
applyBVPO(el, bindings, vnode)
155+
applyPopover(el, bindings, vnode)
161156
}
162157
},
163158
unbind(el) {
164-
removeBVPO(el)
159+
removePopover(el)
165160
}
166161
}

‎src/directives/popover/popover.spec.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import popoverDirective from './popover'
2+
import PopOver from '../../utils/popover.class'
3+
import { mount, createLocalVue as CreateLocalVue } from '@vue/test-utils'
4+
5+
// Key which we use to store tooltip object on element
6+
const BV_POPOVER = '__BV_PopOver__'
7+
8+
describe('v-b-popover directive', () => {
9+
it('should have PopOver class instance', async () => {
10+
const localVue = new CreateLocalVue()
11+
12+
const App = localVue.extend({
13+
directives: {
14+
bPopover: popoverDirective
15+
},
16+
data() {
17+
return {}
18+
},
19+
template: `<button v-b-popover="'content'" title="foobar">button</button>`
20+
})
21+
22+
const wrapper = mount(App, {
23+
localVue: localVue,
24+
attachToDocument: true
25+
})
26+
27+
expect(wrapper.isVueInstance()).toBe(true)
28+
expect(wrapper.is('button')).toBe(true)
29+
const $button = wrapper.find('button')
30+
31+
// Should have instance of popover class on it
32+
expect($button.element[BV_POPOVER]).toBeDefined()
33+
expect($button.element[BV_POPOVER]).toBeInstanceOf(PopOver)
34+
35+
wrapper.destroy()
36+
})
37+
})

0 commit comments

Comments
 (0)