chore(test): implement aria-label for dropdown button and update UI tests
Signed-off-by: Anton Misskii [email protected]
What does this PR do?
implements aria-label for dropdown button and updates UI tests
What issues does this PR fix or reference?
https://github.com/containers/podman-desktop/issues/9532
How to test this PR?
pnpm test:ui
- [ ] Tests are covering the bug fix or the new feature
After reading the comments, it seems that we're not focusing on the right pattern like where to put a name for a button.
IMHO the expectation is to have this component following the combobox semantic while it is not
I should not search for a button but for a combobox using aria-label
so the role 'combobox' should be added (and all the others like aria-expanded, etc)
then for the option, it's the same for the options, I don't see any option role
@deboer-tim why this component has no combobox/option aria-labels ? AFAIK it should be a drop-in replacement from tests framework from Select/Options to Dropdown (tests should not be changed)
After reading the comments, it seems that we're not focusing on the right pattern like where to put a name for a button.
IMHO the expectation is to have this component following the combobox semantic while it is not
I should not search for a button but for a combobox using aria-label
so the role 'combobox' should be added (and all the others like aria-expanded, etc)
Yes, there needs to be a role="combobox" on the same element as aria-label. This will allow code like screen.getByRole('combobox', { name: 'x' }) to work, but not methods userEvent.selectOptions(), which requires at least a couple other aria-labels.
then for the option, it's the same for the options, I don't see any
optionrole@deboer-tim why this component has no combobox/option aria-labels ? AFAIK it should be a drop-in replacement from tests framework from Select/Options to Dropdown (tests should not be changed)
I had a commit that added all the labels as per an example on MDN, but vitest still failed. I went down the rabbit hole of starting to look at vitest source to see if it was actually hardcoded to select, and thought I should look at how UI libraries do it instead. I tried I couple that I had looked at prior to building this - and they literally had no aria support.
The first 10 instances I switched didn't use aria-labels at all and both unit and e2e tests passed, so I assumed it was a problem solvable later...
Short term, yes, let's do this and I'll be trying again for full aria. Long term, we should probably make sure that all locators are part of PR check?