-
Notifications
You must be signed in to change notification settings - Fork 12
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
Update va-select events #351
Conversation
…mponent-library into 671-va-select-event
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know if the team is having an issue with the keydown event or the change one?
I believe it's the |
@Event({ | ||
bubbles: true, | ||
composed: true, | ||
}) | ||
vaSelect: EventEmitter; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, this is what should make that test easier for the app team, right? I was too focused on the keydown
changes to notice this the first time 🙃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests are still failing locally when using userEvent.click(await screen.findByText(/use your current location/i));
. 😞
@@ -79,6 +91,7 @@ const Template = ({ | |||
error={error} | |||
aria-live-region-text={ariaLiveRegionText} | |||
use-add-button={useAddButton} | |||
onKeyDown={() => console.log('KEYDOWN FIRED')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also in favor of this native event + handler replacing the custom one 👍
I have been unable to observe the This code allows you to select a new value using const select = await screen.findByTestId('providersSelect');
userEvent.selectOptions(select, 'Two');
expect(screen.getByRole('option', { name: 'Two' }).selected).to.equal(true); This throws a fireEvent.change(select, { target: { value: 'Two' } }); Related Issuestesting-library/dom-testing-library#413 |
@andresriveratoro can we notate your last comment on the ticket: department-of-veterans-affairs/vets-design-system-documentation#671 and tag Paul so he's aware |
…mponent-library into 671-va-select-event
Chromatic
https://671-va-select-event--60f9b557105290003b387cd5.chromatic.com
Description
Closes department-of-veterans-affairs/vets-design-system-documentation#671
Testing done
E2E, Storybook
Screenshots
Acceptance criteria
vaKeyDown
withkeydown
native eventvaSelect
event propagationDefinition of done