Sd/region check #450
Sd/region check #450
Conversation
# Conflicts: # test/testutils.js
|
@WilcoFiers can you rebase this change onto the merged shadowDOM branch? |
|
We should use |
lib/checks/navigation/region.js
Outdated
| firstLink = node.querySelector('a[href]'); | ||
| const skipLink = getSkiplink(virtualNode); | ||
| const landmarkRoles = aria.getRolesByType('landmark'); | ||
| const implicitLandmarks = landmarkRoles |
There was a problem hiding this comment.
Can you add a comment about how this code works?
lib/checks/navigation/region.js
Outdated
| return firstLink && | ||
| axe.commons.dom.isFocusable(axe.commons.dom.getElementByReference(firstLink, 'href')) && | ||
| firstLink === n; | ||
| // Check if the current element it the skiplink |
There was a problem hiding this comment.
typo in the comment: it => is
test/commons/dom/has-content.js
Outdated
| @@ -77,6 +77,13 @@ describe('dom.hasContent', function () { | |||
| ); | |||
| }); | |||
|
|
|||
| it('is false if noRecurstion is true and the content is not in a child', function () { | |||
| @@ -1,5 +1,23 @@ | |||
| var testUtils = {}; | |||
|
|
|||
| testUtils.MockCheckContext = function () { | |||
test/checks/navigation/region.js
Outdated
| }); | ||
|
|
||
| it('should considered aria labelled elements as content', function () { | ||
| var checkArgs = checkSetup('<div id="target"><div aria-label="axe-core logo"></div><div role="main">Content</div></div>'); |
There was a problem hiding this comment.
Shouldn't this DIV have a role on it for aria-label to be considered content? It isn't exposed consistently for non-interactive elements or landmarks in my experience
There was a problem hiding this comment.
It doesn't matter for the test, but I think you're right. Adding role="img".
| assert.lengthOf(checkContext._relatedNodes, 0); | ||
| }); | ||
|
|
||
| (shadowSupport.v1 ? it : xit)('should ignore skiplink targets inside shadow trees', function () { |
| var div = document.createElement('div'); | ||
| div.innerHTML = '<span id="foo">Content!</span>'; | ||
| var shadow = div.attachShadow({ mode: 'open' }); | ||
| shadow.innerHTML = '<a href="#foo">skiplink</a><div role="main"><slot></slot></div>'; |
There was a problem hiding this comment.
The href is inside the shadow DOM, but the IDref is in the light DOM...? How does that work?
There was a problem hiding this comment.
The href attribute isn't an IDREF attribute. It's a URL, which can only point to light DOM IDs/names. Pretty sure longdesc can't point to things inside shadow DOM for the same reason either.
|
@marcysutton Comments should be resolved. I'll squash moving this into shadowDOM. I had some difficulties rebasing, as it got me conflicts. Wan't too sure how to best get around those. Suggestions? |
|
I usually resolve the conflicts manually when I rebase. But often for a lot of commits it just isn't worth the time, so a squash and merge is easier. |
This closes #389. It reuses parts of #447 and #448.
I found a few problems which this check that I resolved in addition to the shadow DOM work:
<main>)