Skip to content

Add new rule no-duplicate-landmark-elements #1550

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

Merged
merged 5 commits into from
Oct 7, 2020
Merged

Conversation

MelSumner
Copy link
Contributor

@MelSumner MelSumner commented Sep 30, 2020

This is a little bit of a different take on #1327 by @rajasegar
(I tried to commit to that branch but wasn't able to).

MelSumner and others added 2 commits October 7, 2020 10:18
Simplify, simplify, simplify.

Co-authored-by: Melanie Sumner <melaniersumner@gmail.com>
@rwjblue rwjblue force-pushed the require-landmark-labels branch from 22d5b30 to f1a77e9 Compare October 7, 2020 16:50
@@ -0,0 +1,83 @@
'use strict';
Copy link
Member

Choose a reason for hiding this comment

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

Maybe no-duplicate-landmark-elements is a better name?

Comment on lines 9 to 17
const ROLE_LANDMARK_MAP = {
banner: 'header',
main: 'main',
complementary: 'aside',
form: 'form',
search: 'form',
navigation: 'nav',
contentinfo: 'footer',
};
Copy link
Member

Choose a reason for hiding this comment

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

Can you add an inline comment above here linking to the spec where this mapping is defined?

@MelSumner MelSumner changed the title Adds new rule: require-landmark-labels Adds new rule: no-duplicate-landmark-elements Oct 7, 2020
Co-authored-by: Rajasegar Chandran <rajasegar@users.noreply.github.com>
@MelSumner
Copy link
Contributor Author

@rwjblue

I think it's ready to go! 👍🏻

@MelSumner MelSumner requested a review from rwjblue October 7, 2020 18:16
@rwjblue rwjblue merged commit d4b509a into master Oct 7, 2020
@rwjblue rwjblue deleted the require-landmark-labels branch October 7, 2020 20:43
@bmish bmish changed the title Adds new rule: no-duplicate-landmark-elements Add new rule no-duplicate-landmark-elements Oct 11, 2020
@rrenno
Copy link

rrenno commented Nov 20, 2020

Hi @MelSumner I was wondering if you could help clear up some confusion we have about this rule.
For example we have:

    {{#if ....}}
        <div role="alert">
            Failed to build
        </div>
    {{else if .....}}
        ...
        <p role="alert">{{message}}</p>
        ...
    {{else}}
        .....
        <div role="alert">
            {{#if explanationMessage}}
            ....

The rule will flag the multiple uses of role="alert". The 'solution' seems to be adding aria-label/aria-labelledby to each tag, but my question is would the the content of the tags not be sufficient -- e.g the message of the p tag?

Otherwise it seems like we would end up with <p role="alert" aria-label={{message}}>{{message}}</p> which is a bit repetitive. Is this just a limitation of the linter or are we doing something fundamentally at odds with a11y? My understanding is a screen reader would correctly pick up the message.

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

Successfully merging this pull request may close these issues.

None yet

3 participants