-
Notifications
You must be signed in to change notification settings - Fork 104
[DO NOT MERGE] Accessible autocomplete multi-select component #1087
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
base: develop
Are you sure you want to change the base?
[DO NOT MERGE] Accessible autocomplete multi-select component #1087
Conversation
|
Hi @GarvitSinghal47, thanks a lot, I will start going through this week. We'll be in touch. Let me just convert it to draft stage to make a bit more clear to the rest of the team that we use KDS environment in this way only temporarily for development and QA. |
|
Hi @GarvitSinghal47, thanks so much for all this work. This is a solid foundation for keyboard navigation and screen reader support. I also noticed nice touches like highlighting searched text in the options 🙂 I understand some parts are perhaps still in progress. I’m focusing feedback on issue's main focus areas - key a11y behavior, markup structure, and the core architecture. I think it’d best to address the points below in this pull request, so there's no need to refactor core after even more code is added in the upcoming multi-select issues. None of it is urgent, and it’s totally up to you what you want to tackle, based on your time and priorities. I’m ready to jump in too. After we've finished this round, I will invite our a11y expert to test. |
|
Hey @MisRob, I’ve resolved all the comments. Please check when you’re back and let me know if anything else needs to be fixed. |
|
Thank you @GarvitSinghal47. I will re-review everything - likely next week or week after that. |
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.
@GarvitSinghal47 thanks a lot for resolving the feedback! I think this sets a wonderful structure that will make upcoming work easier, and I confirm that those foundational a11y pieces, such as keyboard and focus ring, work well.
In the future, we will definitely QA and fine-tune everything, even though I think it will actually be best to do in the final stage of the component.
As for the component's skeleton stage, this is already large and helpful chunk of work and I think we can consider learningequality/studio#5133 complete 🎉
I will take it from here for some time to play with the next steps in component's development - likely its public API - I don't yet have clarity on how exactly it will look like.
Even though we won't merge this particular PR because this code will be part of Studio at first, and KDS is only for easier testing, know that it'll all be actively used and built upon :)
Description
Do not merge. Will be moved to Studio. We're using KDS environment for easier development and testing.
Part 2: Create public API and fine-tune - WIP @MisRob
learningequality/studio#5429
Part 1: Create skeleton - resolved by @GarvitSinghal47
This PR implements the initial accessible skeleton for an autocomplete multi-select component, as described in #5133. It is part of the broader effort to remove Vuetify from Studio in favor of components built on the Kolibri Design System.
Functionality Included
xon a pillautocompletemode