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

Make landmark rules' description, help messages more consistent #983

Closed
iamrafan opened this issue Jun 29, 2018 · 7 comments
Closed

Make landmark rules' description, help messages more consistent #983

iamrafan opened this issue Jun 29, 2018 · 7 comments
Assignees
Labels

Comments

@iamrafan
Copy link
Contributor

iamrafan commented Jun 29, 2018

As discussed, here are the proposed changes:

1. landmark-banner-is-top-level

a. Description: Ensures the banner landmark is at top level
b. Help: The banner landmark must not be contained in another landmark

2. landmark-contentinfo-is-top-level

a. Description: Ensures the contentinfo landmark is at top level
b. Help: The contentinfo landmark must not be contained in another landmark

3. landmark-main-is-top-level

a. Description: Ensures the main landmark is at top level
b. Help: The main landmark must not be contained in another landmark

4. landmark-no-duplicate-banner

a. Description: Ensures the page has at most one banner landmark
b. Help: The page must not have more than one banner landmark

5 . Check: page-no-duplicate-(banner/contentinfo)

a. Pass: The page does not have more than one (banner/contentinfo) landmark
b. Fail: The page has more than one (banner/contentinfo) landmark

6. landmark-no-duplicate-contentinfo

a. Description: Ensures the page has at most one contentinfo landmark
b. Help: The page must not have more than one contentinfo landmark

7. landmark-one-main

a. Description: Ensures the page has only one main landmark and each iframe in the page has at most one main landmark
b. Help: The page must have one main landmark.

8. Check: page-has-main

a. Pass: The page has at least one main landmark
b. Fail: The page does not have a main landmark

9. Check: page-no-duplicate-main

a. Pass: The page does not have more than one main landmark
b. Fail: The page has more than one main landmark

10. region

a. Description: Ensures all page content is contained by landmarks
b. Help: All page content must be contained by landmarks

11. Check: region

a. Pass: All page content is contained by landmarks
b. Fail: Some page content is not contained by landmarks

axe-core version: 3.0.3
@marcysutton
Copy link
Contributor

Thanks for this @iamrafan! We'll take a closer look.

@iamrafan iamrafan changed the title Modify description, help messages related to landmark rules Make landmark rules' description, help messages more consistent Jun 29, 2018
@WilcoFiers
Copy link
Contributor

We've been trying to keep the help texts relatively short, since those have to fit into a relatively small space in the UI of the extensions. I have no problem updating the pass / fail messages, but making the help texts much longer is may be a problem for the user experience.

@marcysutton
Copy link
Contributor

To put it another way - help text goes into the sidebar of the axe/Attest extensions, so it has to be short. A description can be a little longer.

@iamrafan
Copy link
Contributor Author

iamrafan commented Jul 2, 2018

@marcysutton Any approximate word limit? Do you want "help" text of all the rules above to be shortened?

@marcysutton
Copy link
Contributor

@iamrafan you can look at the existing help text for those rules to get an idea. landmark-one-main is the one in particular that looks too long for the space. The old text fit better: "Page must contain one main landmark"

@iamrafan
Copy link
Contributor Author

@marcysutton @WilcoFiers I shortened the help text, please have a look.

@marcysutton
Copy link
Contributor

It looks like some of the suggestions are basically swapping the help text and description, for which I don't quite understand the purpose. The original intent of this issue was to fix unclear wording or improper English of help and description text.

I found it was difficult to parse the changes without the text side-by-side, so I included both here along with my comments:

1. landmark-banner-is-top-level

Old text:

a. Description: The banner landmark should not be contained in another landmark
b. Help: Banner landmark must be at top level

Propsed text:

a. Description: Ensures the banner landmark is at top level
b. Help: Banner landmark must not be contained in another landmark

Do we really need these changes? Was it a conscious decision to move from "should" to must", given this is a best practice rule?

2. landmark-contentinfo-is-top-level

Old text:

a. Description: The contentinfo landmark should not be contained in another landmark
b. Help: Contentinfo landmark must be at top level

Proposed text:

a. Description: Ensures the contentinfo landmark is at top level
b. Help: The contentinfo landmark must not be contained in another landmark

Same comments/questions as landmark-banner-is-top-level.

3. landmark-main-is-top-level

Old text:

a. Description: The main landmark should not be contained in another landmark
b. Help: Main landmark is not at top level

Proposed text:

a. Description: Ensures the main landmark is at top level
b. Help: The main landmark must not be contained in another landmark

Same as the previous two rules.

4. landmark-no-duplicate-banner

Old text:

a. Description: Ensures the document has no more than one banner landmark
b. Help: Document contain at most one banner landmark

Proposed text:

a. Description: Ensures the page has at most one banner landmark
b. Help: Page must not have more than one banner landmark

The change from "document" to "page" seems fine to me, as long as it still makes sense when iframes are taken into account. I dropped "The" on the proposed help text to make it slightly shorter, since it lives in the axe extension sidebar–also it is more consistent with other rules that reference "Elements should not" or "id must..."

5 . Check: page-no-duplicate-(banner/contentinfo)

Old text:

a. Pass: Document has no more than one banner landmark
b. Fail: Document has more than one banner landmark

Proposed text:

a. Pass: The page does not have more than one banner or contentinfo landmark
b. Fail: The page has more than one banner or contentinfo landmark

I modified the proposed text to read a little easier without the parentheses.

6. landmark-no-duplicate-contentinfo

Old text:

a. Description: Ensures the document has no more than one contentinfo landmark
b. Help: Document contain at most one contentinfo landmark

Proposed text:

a. Description: Ensures the page has at most one contentinfo landmark
b. Help: Page must not have more than one contentinfo landmark

I dropped "The" on the help text but otherwise the consistency does make a lot of sense.

7. landmark-one-main

Old text:

a. Description: Ensures a navigation point to the primary content of the page. If the page contains iframes, each iframe should contain either no main landmarks or just one
b. Help: Page must contain one main landmark

Proposed text:

a. Description: Ensures the page has only one main landmark and each iframe in the page has at most one main landmark
b. Help: Page must have one main landmark.

Dropped "The" on the help text but otherwise good.

8. Check: page-has-main

Old text:

a. Pass: Page has at least one main landmark
b. Fail: Page must have a main landmark

Proposed text:

a. Pass: The page has at least one main landmark
b. Fail: The page does not have a main landmark

9. Check: page-no-duplicate-main

Old text:

a. Pass: Document has no more than one main landmark
b. Fail: Document has more than one main landmark

Proposed text:

a. Pass: The page does not have more than one main landmark
b. Fail: The page has more than one main landmark

10. region

Old text:

a. Description: Ensures all content is contained within a landmark region
b. Help: Content should be contained in a landmark region

Proposed text:

a. Description: Ensures all page content is contained by landmarks
b. Help: All page content must be contained by landmarks

11. Check: region

Old text:

a. Pass: Content contained by ARIA landmark
b. Fail: Content not contained by an ARIA landmark

Proposed text:

a. Pass: All page content is contained by landmarks
b. Fail: Some page content is not contained by landmarks

marcysutton pushed a commit that referenced this issue Jul 12, 2018
WilcoFiers pushed a commit that referenced this issue Oct 4, 2018
…riate (#1156)

A minor fix that was missed in #983 

Closes issue: #983

## Reviewer checks

**Required fields, to be filled out by PR reviewer(s)**
- [x] Follows the commit message policy, appropriate for next version
- [x] Has documentation updated, a DU ticket, or requires no documentation change
- [x] Includes new tests, or was unnecessary
- [x] Code is reviewed for security by: @WilcoFiers
Danidude pushed a commit to tingtun/axe-core that referenced this issue Nov 1, 2018
…riate (dequelabs#1156)

A minor fix that was missed in dequelabs#983 

Closes issue: dequelabs#983

## Reviewer checks

**Required fields, to be filled out by PR reviewer(s)**
- [x] Follows the commit message policy, appropriate for next version
- [x] Has documentation updated, a DU ticket, or requires no documentation change
- [x] Includes new tests, or was unnecessary
- [x] Code is reviewed for security by: @WilcoFiers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants