Skip to content
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

[KSwitch]: Add ariaLabelledBy prop #806

Closed
AlexVelezLl opened this issue Oct 24, 2024 · 9 comments
Closed

[KSwitch]: Add ariaLabelledBy prop #806

AlexVelezLl opened this issue Oct 24, 2024 · 9 comments
Assignees
Labels
Component: KSwitch good first issue Self-contained, straightforward, low-complexity help wanted Open source contributors welcome TAG: a11y type: task Something that needs to be done

Comments

@AlexVelezLl
Copy link
Member

AlexVelezLl commented Oct 24, 2024

Product

Kolibri

Related: learningequality/kolibri#12743

Desired behavior

In some ocasions we want to have the switch input and its label with a different layout that wouldnt be possible to achieve with the current label prop. For example:

Image

And for these cases we should have an aria-labelledby attribute in the input element, and this pointing to the element that has the desired label for the switch. So we should add a new prop to KSwitch named ariaLabelledBy that sets the aria-labelledby attribute of this input element.

Current behavior

Currently we have a label prop/slot, but this is not enough for more complex layouts.

The Value Add

Improved accessibility.

Add labels

Please choose the appropriate label(s) from our existing label list to ensure that your issue is properly categorized. This will help us to better understand and address your issue!

@Sahil-Sinha-11
Copy link
Contributor

Hey @AlexVelezLl I would like to work on this issue.

@AlexVelezLl
Copy link
Member Author

Hey @Sahil-Sinha-11! For sure! I will assign this issue to you :)

@Sahil-Sinha-11
Copy link
Contributor

Hey @AlexVelezLl I added the id and arialabelledBy here
Image

passed it as aprop
Image

and genrated the id here
Image

It gives some error, is this the right way to do it?
Image

@AlexVelezLl
Copy link
Member Author

Hey @Sahil-Sinha-11! No, this aria-labelledBy is for cases where we will not be using the label that is rendered by the KSwitch but rather by an external component. So we dont need to implement a labelId computed prop, nor setting this value to the label or any div tag. We just need to set the aria-labelledby input attribute to the new prop ariaLabelledBy.

So we rather should do:

  <input
    ...
    :aria-labelledby="ariaLabelledBy"
  />

@Sahil-Sinha-11
Copy link
Contributor

Thanks for the guidance @AlexVelezLl.
Should it be done this way?
Image
Image

@AlexVelezLl
Copy link
Member Author

Yes! @Sahil-Sinha-11

@Sahil-Sinha-11
Copy link
Contributor

Hey @AlexVelezLl I made a pr.

@AlexVelezLl
Copy link
Member Author

Thank you @Sahil-Sinha-11! Will review it soon 🤗.

@AlexVelezLl
Copy link
Member Author

Closed by #808

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: KSwitch good first issue Self-contained, straightforward, low-complexity help wanted Open source contributors welcome TAG: a11y type: task Something that needs to be done
Projects
None yet
Development

No branches or pull requests

2 participants