Skip to content

core(a11y): add back html/tags/impact#5439

Merged
patrickhulce merged 2 commits into
masterfrom
more_a11y_details
Jun 11, 2018
Merged

core(a11y): add back html/tags/impact#5439
patrickhulce merged 2 commits into
masterfrom
more_a11y_details

Conversation

@patrickhulce

Copy link
Copy Markdown
Collaborator

Related Issues/PRs
closes #5402

"path": "3,HTML,1,BODY,29,A",
"snippet": "<a href=\"mailto:inbox@email.com\" target=\"_blank\">"
"snippet": "<a href=\"mailto:inbox@email.com\" target=\"_blank\"></a>",
"axeFailureSummary": "Fix all of the following:\n Element is in tab order and does not have accessible text\n\nFix any of the following:\n Element does not have text that is visible to screen readers\n aria-label attribute does not exist or is empty\n aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty or not visible\n Element's default semantics were not overridden with role=\"presentation\"\n Element's default semantics were not overridden with role=\"none\""

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@joelhsmith it seems like in several cases these can be static

Do you have a sense of which cases they're needed per-node? Is it just color-contrast?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking through axe-core, I found these:

aria-allowed-attr
aria-required-children
aria-required-parent
aria-valid-attr-value
aria-valid-attr
color-contrast
fieldset
group-labelledby
duplicate-id
non-empty-if-present

I would not be surprised in the future if even more of them became contextual in the future. Plus people can write their own checks which also might be contextual.

value: rule,
},
details: Audit.makeTableDetails(headings, items),
details: {...Audit.makeTableDetails(headings, items), impact, tags},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should impact and tags be nested or something? I don't really have an opinion, this seems fine and nesting might not be any better, but it may be putting us on a path toward details becoming the new grab bag :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FWIW, I like it where it is.

If it were nested, I can't think of what it would be called. On Deque's site "impact" and "tags" meta is extrapolated to create the "Compliance Data & User Impact" column. "complianceUserImpact:" seems a little long anyway ;-)

@paulirish paulirish left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

seems ok. one question

path: node.path,
snippet: node.snippet,
snippet: node.html || node.snippet,
axeFailureSummary: node.failureSummary,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

how about just summary

@patrickhulce patrickhulce Jun 11, 2018

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

discussed in person, landed on explanation to match with our audit-level naming 👍

@patrickhulce patrickhulce merged commit 4c47fb0 into master Jun 11, 2018
@patrickhulce patrickhulce deleted the more_a11y_details branch June 11, 2018 21:45
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.

Lighthouse 3 Regression in Reporting Data

4 participants