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

selectOptions triggers change twice #917

Closed
ghost opened this issue Apr 8, 2022 · 5 comments · Fixed by #918
Closed

selectOptions triggers change twice #917

ghost opened this issue Apr 8, 2022 · 5 comments · Fixed by #918
Labels
bug Something isn't working released

Comments

@ghost
Copy link

ghost commented Apr 8, 2022

Reproduction example

https://codesandbox.io/s/gracious-tess-xtusee?file=/src/App.test.js

Prerequisites

  1. Render two <select onChange={handleChange}> elements
  2. call selectOptions for one, then another (or call for one then do userEvent.tab())

Expected behavior

onChange is called one time

Actual behavior

onChange is called twice. First time when selecting option, second time when changing option for other <select/> or tabbing out.

User-event version

14.0.4

Additional context

This was not a problem in v13: https://codesandbox.io/s/crazy-bas-1vvrhy?file=/src/App.test.js

@ghost ghost added bug Something isn't working needs assessment This needs to be looked at by a team member labels Apr 8, 2022
@ghost
Copy link
Author

ghost commented Apr 8, 2022

my current walk around to be able to update to v14 is:

function selectOptions(select: HTMLElement, option: string) {
  fireEvent.change(
    select,
    { target: { value: (within(select).getByText(option, { collapseWhitespace: true, selector: 'option' }) as HTMLOptionElement).value } }
  );
}

@ph-fritsche
Copy link
Member

Thanks for the report.

One is dispatched in the utility:

this.dispatchUIEvent(select, 'change')

The other one is from the automatic change event on blur:

document.addEventListener(
'blur',
e => {
const el = e.target as HTMLInputElement
const initialValue = getInitialValue(el)
if (typeof initialValue === 'string' && el.value !== initialValue) {
dispatchUIEvent({} as Config, el, 'change')
}
},
{
capture: true,
passive: true,
},
)

https://codesandbox.io/s/epic-tereshkova-wqodx2?file=/src/App.js

@ph-fritsche ph-fritsche removed the needs assessment This needs to be looked at by a team member label Apr 8, 2022
@ph-fritsche
Copy link
Member

The change event occurs when a new value is committed.
For <input type="checkbox">, <input type="radio">, <input type="file">, <select> this is when the user interaction changes the value.
For date-related input types it looks like only a date selected per date-picker should result in an instant commit. The behavior on edits per e.g. keyboard input might be inconsistent.

Other editable elements with a value property issue a change event when the value was changed and the element loses focus.

But it looks like there was a false assumption on when the browser registers the "initial value" to determine if the value was changed:
While programmatic changes to the value reset the UI value, they don't reset the initial value. They are just ignored in that regard.
The value is compared to the value at the time of the first user input (not the time of the element gaining focus).
(#703 , #747 )

So we might be able to hit two birds with one stone here:
If we set the initial value when we setUIValue, we fix the additional change event when the value is changed by other means than an uncommitted user input - and we also fix missing change events when there is a programmatic change after user input.

document.body.innerHTML=`<input/>`
const input = document.querySelector('input')
input.addEventListener('change', console.log)
input.addEventListener('keydown', e => { if(e.key === 'x') {
  e.target.value += 'X'
  e.preventDefault()
}})

@github-actions
Copy link

🎉 This issue has been resolved in version 14.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ghost
Copy link
Author

ghost commented Apr 11, 2022

Can confirm it fixed broken tests and didn't introduce any new ones. Thanks for fast fix :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant