core: page function updates relating to upcoming tap targets audit #6702
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattzeunert
requested review from
brendankenny,
patrickhulce and
paulirish
as code owners
mattzeunert
force-pushed
the
page-functions-tap-targets
branch
from
030fb89
to
ebeee1b
Compare
mattzeunert
force-pushed
the
page-functions-tap-targets
branch
from
ebeee1b
to
d63af0c
Compare
patrickhulce
reviewed
Dec 3, 2018
| @@ -44,5 +44,10 @@ describe('DetailsRenderer', () => { | |||
| ['style-missing', 'aria-label-missing'] | |||
| ), '<div id="1" style="style" aria-label="label">'); | |||
| }); | |||
|
|
|||
| it('works if attribute values contain line breaks', () => { | |||
| * @returns {string} | ||
| */ | ||
| /* istanbul ignore next */ | ||
| function getNodeSelector(node) { |
There was a problem hiding this comment.
think we could get a quick test for this function? don't need crazy coverage just something to make sure we don't break it
connorjclark
reviewed
Dec 4, 2018
| @@ -45,6 +45,7 @@ function runA11yChecks() { | |||
| // Augment the node objects with outerHTML snippet & custom path string | |||
| // @ts-ignore | |||
| axeResult.violations.forEach(v => v.nodes.forEach(node => { | |||
| // @ts-ignore - getNodePath put into scope via stringification | |||
| node.path = getNodePath(node.element); | |||
There was a problem hiding this comment.
you could keep this the same, if you also export getNodePath in page-functions.js.
Will need that module to export getNodePath for linking nodes in the report for the font-size audit (#6436) (and for other non-accessibility audits).
There was a problem hiding this comment.
I'm not sure what you mean @hoten, it's run in the context of the page and he's exported the stringified version which is what he'll need, no?
There was a problem hiding this comment.
Didn't realize all of that was running in context of the page. Nevermind!
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Splitting up this PR into more digestible bits.
This PR just contains some page function tweaks, and a new function to get an element selector that we can show to the user.
Related Issues/PRs
Tap targets ticket: #4358