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

MWPW-162026 [Milo][Sev2][Catalog] Group of checkboxes is missing FIELDSET … #537

Closed
wants to merge 3 commits into from

Conversation

bozojovicic
Copy link
Contributor

@bozojovicic bozojovicic commented Jan 16, 2025

… element - filter checkboxes

A11Y fix - For Type Desktop/Mobile/Web checkbox group ARIA group/labelledby technique applied to announce the grouping as a part of the checkboxes. This does not work if we have aria-labelledby="elementId" and #elementId is in shadow DOM so I had to create new hidden DIV, outside shadow DOM, which will have that ID and contain the test "Types".

Test page : /products/catalog

Resolves: MWPW-162026

Test URLs:

@bozojovicic bozojovicic requested a review from a team as a code owner January 16, 2025 13:50
Copy link

aem-code-sync bot commented Jan 16, 2025

Page Scores Audits Google
📱 /?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@bozojovicic bozojovicic marked this pull request as draft January 16, 2025 14:03
@milo-pr-merge-cc
Copy link

Skipped merging 537: MWPW-162026 [Milo][Sev2][Catalog] Group of checkboxes is missing FIELDSET … due to missing verified label. kindly make sure that the PR has been verified

1 similar comment
@milo-pr-merge-cc
Copy link

Skipped merging 537: MWPW-162026 [Milo][Sev2][Catalog] Group of checkboxes is missing FIELDSET … due to missing verified label. kindly make sure that the PR has been verified

@bozojovicic bozojovicic marked this pull request as ready for review January 20, 2025 09:50
@milo-pr-merge-cc
Copy link

Skipped merging 537: MWPW-162026 [Milo][Sev2][Catalog] Group of checkboxes is missing FIELDSET … due to missing verified label. kindly make sure that the PR has been verified

@@ -85,6 +103,7 @@ export default async function init(el) {
const sidenav = await initSidenav(sidenavEl);
el.prepend(sidenav);
await sidenav.updateComplete;
addCheckboxGroupAttributes(sidenav);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Agreed. And again, last week I tried to do that and I gave up and now I don't remember why. I created Milo PR for that adobecom/milo#3518 and it works quite nice. So closing this one.

@bozojovicic
Copy link
Contributor Author

Implemented in Milo adobecom/milo#3518
CC @yesil

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

Successfully merging this pull request may close these issues.

2 participants