fix(landmark-unique): correct form/section landmark calculation#5086
fix(landmark-unique): correct form/section landmark calculation#5086WilcoFiers wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Updates landmark-unique-matches to rely on the shared ARIA role-type logic for landmark detection, fixing incorrect matching for <section>/<form> when explicit non-landmark roles are present and ensuring explicit landmark roles are handled consistently.
Changes:
- Replace custom landmark detection in
lib/rules/landmark-unique-matches.jswithgetRoleType(vNode) === 'landmark'. - Refactor and expand tests for
<section>/<form>matching behavior, including explicit landmark and explicit non-landmark role cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/rules/landmark-unique-matches.js | Simplifies landmark matching by using the central getRoleType logic, aligning behavior with existing implicit-role/name rules. |
| test/rule-matches/landmark-unique-matches.js | Refactors the section/form tests into per-element suites and adds coverage for explicit landmark vs non-landmark roles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| describe(`${elementType} must have accessible names to be matched`, () => { | ||
| it(`should match with a label`, () => { |
There was a problem hiding this comment.
The describe title says the element "must have accessible names to be matched", but this suite also includes a case that should match without a label when an explicit landmark role is present. Consider renaming the suite to reflect the actual behavior being tested (implicit section/form landmark mapping requires a name; explicit landmark roles match regardless of name) to avoid confusion when reading test output.
|
Didn't mean to open this as. PR. I was just testing. |
The landmark-unique-matches function used an outdated approach to finding landmarks. I noticed that looking at #5085, which tried to patch the problem. The better solve is to use the existing code for finding larndmarks.
Closes: #4722, closes #5064