Skip to content

Commit 076cf37

Browse files
kentcdoddskentcdodds
committed
fix(selectOptions): improve correctness
1 parent efe87aa commit 076cf37

File tree

4 files changed

+74
-64
lines changed

4 files changed

+74
-64
lines changed

‎src/user-event/__tests__/helpers/utils.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,13 @@ const changeLabelGetter = {
232232
before.checked ? 'checked' : 'unchecked',
233233
after.checked ? 'checked' : 'unchecked',
234234
].join(' -> '),
235+
selectedOptions: ({before, after}) => {
236+
const beforeString = JSON.stringify(before.selectedOptions)
237+
const afterString = JSON.stringify(after.selectedOptions)
238+
return beforeString === afterString
239+
? null
240+
: [beforeString, afterString].join(' -> ')
241+
},
235242
}
236243
changeLabelGetter.selectionStart = changeLabelGetter.value
237244
changeLabelGetter.selectionEnd = changeLabelGetter.value
@@ -248,7 +255,7 @@ function getChanges({before, after}) {
248255
}
249256
}
250257

251-
return Array.from(changes).join('\n')
258+
return Array.from(changes).filter(Boolean).join('\n')
252259
}
253260

254261
// eslint-disable-next-line jest/prefer-hooks-on-top

‎src/user-event/__tests__/select-options.js

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,34 +3,29 @@ import {setupSelect, addListeners} from './helpers/utils'
33

44
test('fires correct events', async () => {
55
const {select, options, getEventSnapshot} = setupSelect()
6-
await userEvent.selectOptions(select, '1')
6+
await userEvent.selectOptions(select, '2')
77
expect(getEventSnapshot()).toMatchInlineSnapshot(`
8-
Events fired on: select[name="select"][value="1"]
8+
Events fired on: select[name="select"][value="2"]
99
10+
select[name="select"][value="1"] - pointerover
11+
select[name="select"][value="1"] - pointerenter
12+
select[name="select"][value="1"] - mouseover: Left (0)
13+
select[name="select"][value="1"] - mouseenter: Left (0)
14+
select[name="select"][value="1"] - pointermove
15+
select[name="select"][value="1"] - mousemove: Left (0)
1016
select[name="select"][value="1"] - pointerdown
11-
selectedOptions: ["1"] -> ["1"]
1217
select[name="select"][value="1"] - mousedown: Left (0)
13-
selectedOptions: ["1"] -> ["1"]
1418
select[name="select"][value="1"] - focus
1519
select[name="select"][value="1"] - focusin
16-
selectedOptions: ["1"] -> ["1"]
1720
select[name="select"][value="1"] - pointerup
18-
selectedOptions: ["1"] -> ["1"]
1921
select[name="select"][value="1"] - mouseup: Left (0)
20-
selectedOptions: ["1"] -> ["1"]
2122
select[name="select"][value="1"] - click: Left (0)
22-
selectedOptions: ["1"] -> ["1"]
23-
option[value="1"] - mouseover: Left (0)
24-
option[value="1"] - mousemove: Left (0)
25-
option[value="1"] - mousedown: Left (0)
26-
option[value="1"] - mouseup: Left (0)
27-
option[value="1"] - click: Left (0)
28-
select[name="select"][value="1"] - change
29-
selectedOptions: ["1"] -> ["1"]
23+
select[name="select"][value="2"] - input
24+
select[name="select"][value="2"] - change
3025
`)
3126
const [o1, o2, o3] = options
32-
expect(o1.selected).toBe(true)
33-
expect(o2.selected).toBe(false)
27+
expect(o1.selected).toBe(false)
28+
expect(o2.selected).toBe(true)
3429
expect(o3.selected).toBe(false)
3530
})
3631

@@ -40,33 +35,34 @@ test('fires correct events on multi-selects', async () => {
4035
expect(getEventSnapshot()).toMatchInlineSnapshot(`
4136
Events fired on: select[name="select"][value=["1","3"]]
4237
43-
select[name="select"][value=[]] - pointerdown
44-
selectedOptions: [] -> []
45-
select[name="select"][value=[]] - mousedown: Left (0)
46-
selectedOptions: [] -> []
47-
select[name="select"][value=[]] - focus
48-
select[name="select"][value=[]] - focusin
49-
selectedOptions: [] -> []
50-
select[name="select"][value=[]] - pointerup
51-
selectedOptions: [] -> []
52-
select[name="select"][value=[]] - mouseup: Left (0)
53-
selectedOptions: [] -> []
54-
select[name="select"][value=[]] - click: Left (0)
55-
selectedOptions: [] -> []
38+
option[value="1"] - pointerover
39+
select[name="select"][value=[]] - pointerenter
5640
option[value="1"] - mouseover: Left (0)
41+
select[name="select"][value=[]] - mouseenter: Left (0)
42+
option[value="1"] - pointermove
5743
option[value="1"] - mousemove: Left (0)
44+
option[value="1"] - pointerdown
5845
option[value="1"] - mousedown: Left (0)
46+
select[name="select"][value=[]] - focus
47+
select[name="select"][value=[]] - focusin
48+
option[value="1"] - pointerup
5949
option[value="1"] - mouseup: Left (0)
60-
option[value="1"] - click: Left (0)
50+
select[name="select"][value=["1"]] - input
6151
select[name="select"][value=["1"]] - change
62-
selectedOptions: ["1"] -> ["1"]
52+
option[value="1"] - click: Left (0)
53+
option[value="3"] - pointerover
54+
select[name="select"][value=["1"]] - pointerenter
6355
option[value="3"] - mouseover: Left (0)
56+
select[name="select"][value=["1"]] - mouseenter: Left (0)
57+
option[value="3"] - pointermove
6458
option[value="3"] - mousemove: Left (0)
59+
option[value="3"] - pointerdown
6560
option[value="3"] - mousedown: Left (0)
61+
option[value="3"] - pointerup
6662
option[value="3"] - mouseup: Left (0)
67-
option[value="3"] - click: Left (0)
63+
select[name="select"][value=["1","3"]] - input
6864
select[name="select"][value=["1","3"]] - change
69-
selectedOptions: ["1","3"] -> ["1","3"]
65+
option[value="3"] - click: Left (0)
7066
`)
7167
const [o1, o2, o3] = options
7268
expect(o1.selected).toBe(true)

‎src/user-event/__tests__/toggle-selectoptions.js

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,25 +10,18 @@ test('should fire the correct events for multiple select', async () => {
1010
Events fired on: select[name="select"][value=["1"]]
1111
1212
select[name="select"][value=[]] - pointerdown
13-
selectedOptions: [] -> []
1413
select[name="select"][value=[]] - mousedown: Left (0)
15-
selectedOptions: [] -> []
1614
select[name="select"][value=[]] - focus
1715
select[name="select"][value=[]] - focusin
18-
selectedOptions: [] -> []
1916
select[name="select"][value=[]] - pointerup
20-
selectedOptions: [] -> []
2117
select[name="select"][value=[]] - mouseup: Left (0)
22-
selectedOptions: [] -> []
2318
select[name="select"][value=[]] - click: Left (0)
24-
selectedOptions: [] -> []
2519
option[value="1"] - mouseover: Left (0)
2620
option[value="1"] - mousemove: Left (0)
2721
option[value="1"] - mousedown: Left (0)
2822
option[value="1"] - mouseup: Left (0)
2923
option[value="1"] - click: Left (0)
3024
select[name="select"][value=["1"]] - change
31-
selectedOptions: ["1"] -> ["1"]
3225
`)
3326

3427
expect(form).toHaveFormValues({select: ['1']})

‎src/user-event/select-options.js

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,41 @@
11
import {wrapAsync} from '../wrap-async'
2+
import {createEvent} from '../events'
23
import {getConfig} from '../config'
3-
import {fireEvent, getMouseEventOptions} from './utils'
4-
import {clickElement} from './click'
4+
import {fireEvent} from './utils'
5+
import {click} from './click'
6+
import {focus} from './focus'
57

68
async function selectOption(select, option, init) {
7-
await fireEvent.mouseOver(option, getMouseEventOptions('mouseover', init))
8-
await fireEvent.mouseMove(option, getMouseEventOptions('mousemove', init))
9-
await fireEvent.mouseDown(option, getMouseEventOptions('mousedown', init))
10-
await fireEvent.focus(option)
11-
await fireEvent.mouseUp(option, getMouseEventOptions('mouseup', init))
12-
await fireEvent.click(option, getMouseEventOptions('click', init, 1))
13-
149
option.selected = true
10+
await fireEvent(select, createEvent('input', select, init))
11+
await fireEvent(select, createEvent('change', select, init))
12+
}
1513

16-
await fireEvent.change(select)
14+
async function selectOptionInMultiple(select, options, init) {
15+
for (const option of options) {
16+
// events fired for multiple select are weird. Can't use hover...
17+
await fireEvent.pointerOver(option, init)
18+
await fireEvent.pointerEnter(select, init)
19+
await fireEvent.mouseOver(option)
20+
await fireEvent.mouseEnter(select)
21+
await fireEvent.pointerMove(option, init)
22+
await fireEvent.mouseMove(option, init)
23+
await fireEvent.pointerDown(option, init)
24+
await fireEvent.mouseDown(option, init)
25+
await focus(select, init)
26+
await fireEvent.pointerUp(option, init)
27+
await fireEvent.mouseUp(option, init)
28+
await selectOption(select, option, init)
29+
await fireEvent.click(option, init)
30+
}
1731
}
1832

19-
async function selectOptions(element, values, init) {
20-
await clickElement(element, init)
33+
async function selectOptionInSingle(select, option, init) {
34+
await click(select, init)
35+
await selectOption(select, option, init)
36+
}
2137

38+
async function selectOptions(element, values, init) {
2239
const valArray = Array.isArray(values) ? values : [values]
2340
const allOptions = Array.from(element.querySelectorAll('option'))
2441
const selectedOptions = valArray.map(val => {
@@ -38,17 +55,14 @@ async function selectOptions(element, values, init) {
3855
})
3956

4057
if (element.multiple) {
41-
for (const option of selectedOptions) {
42-
await selectOption(element, option)
43-
}
58+
await selectOptionInMultiple(element, selectedOptions, init)
59+
} else if (selectedOptions.length === 1) {
60+
await selectOptionInSingle(element, selectedOptions[0], init)
4461
} else {
45-
if (selectedOptions.length !== 1) {
46-
throw getConfig().getElementError(
47-
`Cannot select multiple options on a non-multiple select`,
48-
element,
49-
)
50-
}
51-
await selectOption(element, selectedOptions[0])
62+
throw getConfig().getElementError(
63+
`Cannot select multiple options on a non-multiple select`,
64+
element,
65+
)
5266
}
5367
}
5468
selectOptions = wrapAsync(selectOptions)

0 commit comments

Comments
 (0)