IBMa / equal-access

IBM Equal Access Accessibility Checker contains tools to automate accessibility checking from a browser or in a continuous development/build environment
https://www.ibm.com/able/toolkit/tools#develop
Apache License 2.0
635 stars 82 forks source link

[BUG] Landmark label uniqueness should not be enforced across dialog boundaries - 3610 #599

Closed aliunwala closed 2 years ago

aliunwala commented 2 years ago

Opened by Tom:

What tool(s) are you using?

See example: https://codesandbox.io/s/issue-3610-c0gjr

Typically, dialogs are considered as modal, so components within the dialog are logically separated from components outside of the dialog. They're somewhat a separate page / viewport within the page. Label uniqueness should be enforced within the same context (dialog / page outside dialogs) and not between the main page and the dialog.

Link to original issue: https://github.ibm.com/IBMa/E2E/issues/3610

Image of original issue: image

aliunwala commented 2 years ago

Comment from Phil: @aliunwala Is there anything in the current rule implementation:

[Rule Impl] pt3 Nested Landmarks - This is a new rule that deals with failure cases around nested landmarks found the in DOM #3775

that will prevent this new issue to not enforce label uniqueness across dialog boundaries? I think #3775 is only dealing with common parents and labels for the same type landmarks and not considering dialogs.
In other words, how is the #3775 implementation considering dialogs?

aliunwala commented 2 years ago

@philljenkins Do dialogs ever have multiple landmarks inside them? I feel like they could but I have a gut feeling that this must be rare.

aliunwala commented 2 years ago

If dialog's with landmarks are rare we could just turn off landmark_name_unique when we are under a dialog and call this ticket done for now.

However, If we want to check for landmark uniqueness not only on the main page but under each and every instance of dialogs anywhere on the page this rule will need some serious rule rewriting to include the dialog case into this rule. Or even possibly a new rule to specifically cover the dialog without making the landmark MDX help file extra confusing.

philljenkins commented 2 years ago

@philljenkins Do dialogs ever have multiple landmarks inside them? I feel like they could but I have a gut feeling that this must be rare.

yes they might, but I do not know if it is rare for sure. I agree its probably rare because dialogs do not usually have a lot of content for which landmarks would be common.

@aliunwala We can open a new issue to deal with checking for landmark uniquesness.

philljenkins commented 2 years ago

However, If we want to check for landmark uniqueness not only on the main page but under each and every instance of dialogs anywhere on the page this rule will need some serious rule rewriting to include the dialog case into this rule...

Since this current rule doesn't check for landmarks within dialogs, then this issuee should be closed. We do NOT want to inforce uniqueness across (between) dialogs and the main page. This issue was opend to check if that was true.

aliunwala commented 2 years ago

Going to continue this work into next sprint. As this needs some rule logic changes.

aliunwala commented 2 years ago

Found a weird bug that is possibly blocking this work. We are seeing the nodes nested under dialog not trigger the rule context. Following up with Tom to figure out what is the expected behavior.

aliunwala commented 2 years ago

Moving this into review as the implementation work is done.

marcjohlic commented 2 years ago

Erick is working on a build issue