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
feat(coral): use Combobox
instead of NativeSelect
for selecting topic names
#2101
Conversation
Signed-off-by: Mathieu Anderson <mathieu.anderson@aiven.io>
Signed-off-by: Mathieu Anderson <mathieu.anderson@aiven.io>
Signed-off-by: Mathieu Anderson <mathieu.anderson@aiven.io>
Signed-off-by: Mathieu Anderson <mathieu.anderson@aiven.io>
It's this line:
Combobox does not have use a
The placeholder was added only for design purposes for select really. So removing it here is fine. All in all -> I'm not sure if this combox is accessible. I had a hard time figuring out how to use it with a screenreader and I could look at it to help me understand it. I think we should check how that's usually done and recommended and see if we can support users with screen reader better. Otherwise we're improving UX for some user by removing usability for others. The fact that testing library has a problem figuring that element out is probably also not a good sign in that regard. |
Signed-off-by: Mathieu Anderson <mathieu.anderson@aiven.io>
The Additionally, there was some wrong copy pasting action in the This commit seeks to address these issues, and I don't see the warnings any more locally. 2b3a266 |
Ajh lol @programmiri you commenetd about what I just pushed a commit to fix ^^ |
Signed-off-by: Mathieu Anderson <mathieu.anderson@aiven.io>
Closing as this warrants more conversation and possibly a11y fixes to the |
Linked issue
Resolves: #2099
What kind of change does this PR introduce?
What is the current behavior?
We use
NativeSelect
for Topic name fields. There can be many more topic names than is comfortabnle for the native select UI.What is the new behavior?
Combobox
component toForm
Combobox
for Topic name fieldsScreen.Recording.2023-12-08.at.14.02.06.mov
Other information
react-testing-library
do not work withCombobox
, so we need to imperatively click on the element to access options nowconsole.error
fromdownshift
has popped up in the test reports. Not sure how to address it, as I'm not even sure how the component is switching from controlled to uncontrolled.Combobox
use an actual placeholder instead of an option for the empty state (color of the content differs). This may also introduce a11y concern, as I think the "empty option" was added to improve a11y.Requirements (all must be checked before review)
main
branch have been pulledpnpm lint
has been run successfully