Using the new ariaLabelledBy prop in KSwitch #12918
Conversation
github-actions
bot
added
APP: Coach
|
Thanks for yet another contribution @RONAK-AI647 :)! I will invite my colleagues @LianaHarris360 or @marcellamaki for review. One thing I noticed myself concerns translations of strings. Please have a look at how to write localized strings and try to follow the guide. |
Build Artifacts
|
|
Hi @RONAK-AI647 - thanks for your contribution! The main goal of using aria-labeledby is to associate text that is present on the page to another element. It's usually applied in specific scenarios where something is visually apparent, but semantically within the HTML, would not be properly associated with someone who is using a screen reader. What text in the UI are you trying to associate with each of these elements? How might you do that without adding additional spans and text? 
|
| <KSwitch | ||
| name="toggle-quiz-visibility" | ||
| :aria-labelledby="'toggle-quiz-visibility'" |
There was a problem hiding this comment.
Should this line match the id='toggle-quiz-visibility-label' in the span above?
There was a problem hiding this comment.
Also, shouldn't the prop be named ariaLabelledBy rather than :aria-labelledby?
There was a problem hiding this comment.
Some great thoughts have already been shared, I just had a couple more points to add. Thanks for your work on this @RONAK-AI647 !
| v-else | ||
| :key="`switch-${lesson.id}`" | ||
| v-else: | ||
| key="`switch-${lesson.id}`" |
There was a problem hiding this comment.
I believe this piece of code is causing an error in my code editor. Is there a particular reason that the : colon was moved from in front of key and positioned after v-else?
| name="toggle-lesson-visibility" | ||
| :ariaLabelledBy="`toggle-lesson-visibility-label-${lesson.id}`" | ||
| label="" |
There was a problem hiding this comment.
Since it is set as an empty string, is the label property still required now that we have the ariaLabelledBy prop in use?
| <KSwitch | ||
| name="toggle-quiz-visibility" | ||
| :aria-labelledby="'toggle-quiz-visibility'" |
There was a problem hiding this comment.
Also, shouldn't the prop be named ariaLabelledBy rather than :aria-labelledby?
|
Thank You everybody for reviewing @marcellamaki @LianaHarris360 @nucleogenesis @MisRob , give me some time , i will return with everything resolved .!!!!!! |
Summary
1)There are many uses of Kswitch in kolibri's codebase. learningequality/kolibri-design-system#806
introduced a new feature named arialLabelledBy in KDS , using that feature , some of the Kswitchs' doesnt have a label set.
3)All KSwitch components should have a label, either set by the ariaLabelledBy or by the label prop
References
#12743
Issue/learningequality/kolibri-design-system#806
…
Reviewer guidance
…