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

Attributes removed from elements #110

Open
ramono opened this issue Mar 19, 2020 · 3 comments
Open

Attributes removed from elements #110

ramono opened this issue Mar 19, 2020 · 3 comments

Comments

@ramono
Copy link

ramono commented Mar 19, 2020

Recently I've submitted a PR to silverstripe/framework to add a missing attribute needed for accessibility: silverstripe/silverstripe-framework#9427

But today using it in another project I've realized that the new attribute is not being added, because this module is overwriting certain templates.

Moreover, on CheckboxSetField.ss the $AttributesHTML variable is removed in favor of hardcoded individual attributes, is this really necessary?

Would it be possible to leave the $AttributesHTML variable and add the conditional after, so any extra attributes added to the field don't get lost? (Specifically asking if the name attribute is needed when display logic is used, or if it can be changed to a different data attribute instead).

Besides that, I'd like to add the role="option" to the list elements so it appears as per my PR

Thanks

@michalkleiner
Copy link
Collaborator

Hello! Thank you for raising the issue and apologies it hasn't been responded to earlier.

We can look into the custom attributes vs the $AttributesHTML variable. If your issue is with the missing role attribute, if you opened a PR adding just that I'd be happy to review it.

@michalkleiner
Copy link
Collaborator

This PR might be relevant to this issue, would it solve your issue sufficiently? #137

@ramono
Copy link
Author

ramono commented Feb 15, 2023

Hi, thanks for checking into it, it looks like the #137 solves the problem, happy for this issue to be closed once that is merged 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants