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

fix(NcCheckboxRadioSwitch): improve rendering and prevent unecessary elements #5001

Merged
merged 1 commit into from Jan 2, 2024

Conversation

skjnldsv
Copy link
Contributor

  • Make the icon inert
  • Do not render the content text if undefined

@skjnldsv skjnldsv added bug Something isn't working 3. to review Waiting for reviews labels Dec 28, 2023
@skjnldsv skjnldsv added this to the 8.4.1 milestone Dec 28, 2023
@skjnldsv skjnldsv self-assigned this Dec 28, 2023
@ShGKme
Copy link
Contributor

ShGKme commented Dec 28, 2023

I don't understand... Icon span is neither focusable nor interactive anyway, does inert change anything here?

Is it an alternative or a part of #4999 ?

@emoral435
Copy link
Contributor

From what I am seeing, the inert attribute would just be a preventative in case the inserted slot is indeed focusable, no? It is not a breaking change, more of a preventative one from what I gather. As of now, I do not believe it hurts to add it on

@ShGKme
Copy link
Contributor

ShGKme commented Dec 28, 2023

the inert attribute would just be a preventative in case the inserted slot is indeed focusable, no?

It would be an a11y issue if there was an interactive focusable and yet inert element...

@skjnldsv skjnldsv changed the title fix(NcCheckboxRadioSwitch): fix shift+click on firefox fix(NcCheckboxRadioSwitch): improve rendering and prevent unecessary elements Dec 29, 2023
@skjnldsv
Copy link
Contributor Author

skjnldsv commented Dec 29, 2023

It would be an a11y issue if there was an interactive focusable and yet inert element...

That's the point, sanitising what we allow in those slots.
It also hides the element and its content from assistive technologies by excluding them from the accessibility tree. Which it should as it's a non descriptive image and should not be read or described. It's basically what aria-hidden is doing (already here), but stronger imho

Is it an alternative or a part of #4999 ?

No, It just happen that I branched it from the other PR, it will change its base branch once #4999 is merged

@skjnldsv skjnldsv force-pushed the fix/inert-checkbox-radio-switch branch from 34a1a7f to 9511e3f Compare January 2, 2024 08:52
Base automatically changed from fix/checkbox-label-shift-click-FF to master January 2, 2024 08:55
…elements

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the fix/inert-checkbox-radio-switch branch from 9511e3f to 54851fd Compare January 2, 2024 17:16
@skjnldsv skjnldsv merged commit 087c065 into master Jan 2, 2024
15 checks passed
@skjnldsv skjnldsv deleted the fix/inert-checkbox-radio-switch branch January 2, 2024 17:33
@susnux susnux modified the milestones: 8.4.1, 8.5.0 Jan 15, 2024
@Pytal Pytal mentioned this pull request Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants