-
Notifications
You must be signed in to change notification settings - Fork 784
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
feat(new-rule): Add WCAG 2.2 target-size rule #3616
Changes from 1 commit
7d9b70c
54817d7
80299b2
302db49
0172fbe
5b1c1a3
a0eaaf8
c4029a5
55b037c
81feac6
eb27049
217ac64
10b8972
297524f
a25d7a0
f6b97df
8ab55c7
496f227
8ea60ec
a5a0512
368d612
b4462e5
9e7e9b6
4fefa21
7885fe8
840cf4e
1e204c1
b2f12e8
17ce8ea
d72b683
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -324,11 +324,13 @@ function findScrollRegionParent(vNode, parentVNode) { | |
*/ | ||
function addNodeToGrid(grid, vNode) { | ||
const gridSize = constants.gridSize; | ||
// save a reference to where this element is in the grid so we | ||
// can find it even if it's in a subgrid | ||
vNode._grid = grid; | ||
|
||
vNode.clientRects.forEach(rect => { | ||
if (rect.right <= 0 || rect.bottom <= 0) { | ||
return; | ||
} | ||
// save a reference to where this element is in the grid so we | ||
// can find it even if it's in a subgrid | ||
vNode._grid ??= grid; | ||
Comment on lines
+328
to
+333
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMPORTANT: These lines have been modified to fix a bug where elements outside the viewport could still be added to the grid. |
||
const x = rect.left; | ||
const y = rect.top; | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,33 +1,39 @@ | ||||||
import createGrid from './create-grid'; | ||||||
|
||||||
function findNearbyElms(vNode, margin = 0) { | ||||||
export default function findNearbyElms(vNode, margin = 0) { | ||||||
/*eslint no-bitwise: 0*/ | ||||||
const gridSize = createGrid(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So <script>function clicked() { console.log('click') }</script>
<span class="failed h2 w3"></span>
<span style="opacity: 0" class="failed h2 w3" onclick="clicked()"></span>
<span class="failed h2 w3"></span> Both of the visible spans pass the target-size rule, but the opacity 0 span is still clickable, meaning that it still would cause a problem of clicking the wrong element when too close together. I would imagine this should fail the rule still. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another question: How should we handle things on-top of these nodes? For example, taking the following code (modified B4 example again): <style>
.foo {
width: 400px;
height: 400px;
position: absolute !important;
background: red;
top: -5px;
z-index: 2;
}
</style>
<div>
<div class="foo"></div>
<span class="failed h2 w3"></span>
<span class="failed h2 w3"></span>
<span class="failed h2 w3"></span>
</div> The nodes still fail even though it's not possible to click on them. Same if a modal is currently open. Should we fail these nodes still? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ended up doing the overlap thing in the |
||||||
const neighbors = []; | ||||||
if (!vNode._grid?.cells?.length) { | ||||||
return []; // Elements not in the grid don't have ._grid | ||||||
} | ||||||
|
||||||
const rect = vNode.boundingClientRect; | ||||||
const gridCells = vNode._grid.cells; | ||||||
const boundaries = { | ||||||
topRow: ((rect.top - margin) / gridSize) | 0, | ||||||
bottomRow: ((rect.bottom + margin) / gridSize) | 0, | ||||||
leftCol: ((rect.left - margin) / gridSize) | 0, | ||||||
rightCol: ((rect.right + margin) / gridSize) | 0 | ||||||
}; | ||||||
|
||||||
const topRow = ((rect.top - margin) / gridSize) | 0; | ||||||
const bottomRow = ((rect.bottom + margin) / gridSize) | 0; | ||||||
const leftCol = ((rect.left - margin) / gridSize) | 0; | ||||||
const rightCol = ((rect.right + margin) / gridSize) | 0; | ||||||
const neighbors = []; | ||||||
loopGridCells(gridCells, boundaries, vNeighbor => { | ||||||
if (vNeighbor && vNeighbor !== vNode && !neighbors.includes(vNeighbor)) { | ||||||
neighbors.push(vNeighbor); | ||||||
} | ||||||
}); | ||||||
return neighbors; | ||||||
} | ||||||
|
||||||
function loopGridCells(gridCells, boundaries, cb) { | ||||||
const { topRow, bottomRow, leftCol, rightCol } = boundaries; | ||||||
for (let row = topRow; row <= bottomRow; row++) { | ||||||
for (let col = leftCol; col <= rightCol; col++) { | ||||||
for (let i = 0; i <= gridCells[row][col].length; i++) { | ||||||
var vNeighbour = gridCells[row][col][i]; | ||||||
// Avoid duplication | ||||||
if ( | ||||||
vNeighbour && | ||||||
vNeighbour !== vNode && | ||||||
!neighbors.includes(vNeighbour) | ||||||
) { | ||||||
neighbors.push(vNeighbour); | ||||||
} | ||||||
// Don't loop on elements outside the grid | ||||||
const length = gridCells[row]?.[col]?.length ?? -1; | ||||||
for (let i = 0; i <= length; i++) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
cb(gridCells[row][col][i]); | ||||||
} | ||||||
} | ||||||
} | ||||||
return neighbors; | ||||||
} | ||||||
|
||||||
export default findNearbyElms; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,148 @@ | ||
// Additional tests for createGrid are part of createRectStack tests, | ||
// which is what createGrid was originally part of | ||
describe('create-grid', function () { | ||
var fixture; | ||
var createGrid = axe.commons.dom.createGrid; | ||
var fixtureSetup = axe.testUtils.fixtureSetup; | ||
|
||
function findPositions(grid, vNode) { | ||
var positions = []; | ||
grid.cells.forEach(function (rowCells, rowIndex) { | ||
rowCells.forEach(function (cells, colIndex) { | ||
if (cells.includes(vNode)) { | ||
positions.push({ x: rowIndex, y: colIndex }); | ||
} | ||
}); | ||
}); | ||
return positions; | ||
} | ||
|
||
it('returns the grid size', function () { | ||
axe.setup(); | ||
assert.equal(createGrid(), axe.constants.gridSize); | ||
}); | ||
|
||
it('sets ._grid to nodes in the grid', function () { | ||
fixture = fixtureSetup('<span>Hello world</span>'); | ||
assert.isUndefined(fixture._grid); | ||
assert.isUndefined(fixture.children[0]._grid); | ||
|
||
createGrid(); | ||
assert.isDefined(fixture._grid); | ||
assert.equal(fixture._grid, fixture.children[0]._grid); | ||
}); | ||
|
||
it('adds elements to the correct cell in the grid', function () { | ||
fixture = fixtureSetup('<span>Hello world</span>'); | ||
createGrid(); | ||
var positions = findPositions(fixture._grid, fixture.children[0]); | ||
assert.deepEqual(positions, [{ x: 0, y: 0 }]); | ||
}); | ||
|
||
it('adds large elements to multiple cell', function () { | ||
fixture = fixtureSetup( | ||
'<span style="display: inline-block; width: 300px; height: 300px;">' + | ||
'Hello world</span>' | ||
); | ||
createGrid(); | ||
|
||
var positions = findPositions(fixture._grid, fixture.children[0]); | ||
assert.deepEqual(positions, [ | ||
{ x: 0, y: 0 }, | ||
{ x: 0, y: 1 }, | ||
{ x: 1, y: 0 }, | ||
{ x: 1, y: 1 } | ||
]); | ||
}); | ||
|
||
describe('hidden elements', function () { | ||
beforeEach(function () { | ||
// Ensure the fixture itself is part of the grid, even if its content isn't | ||
document | ||
.querySelector('#fixture') | ||
.setAttribute('style', 'min-height: 10px'); | ||
}); | ||
|
||
it('does not add hidden elements', function () { | ||
fixture = fixtureSetup('<div style="display: none">hidden</div>'); | ||
createGrid(); | ||
var position = findPositions(fixture._grid, fixture.children[0]); | ||
assert.isEmpty(position); | ||
assert.isUndefined(fixture.children[0]._grid); | ||
}); | ||
|
||
it('does not add off screen elements', function () { | ||
fixture = fixtureSetup( | ||
'<div style="position: fixed; top: -3em">off screen</div>' | ||
); | ||
createGrid(); | ||
var position = findPositions(fixture._grid, fixture.children[0]); | ||
assert.isEmpty(position); | ||
assert.isUndefined(fixture.children[0]._grid); | ||
}); | ||
|
||
it('does add partially on screen elements', function () { | ||
fixture = fixtureSetup( | ||
'<div style="position: fixed; top: -1em; min-height: 2em">off screen</div>' | ||
); | ||
createGrid(); | ||
var position = findPositions(fixture._grid, fixture.children[0]); | ||
assert.deepEqual(position, [{ x: 0, y: 0 }]); | ||
}); | ||
}); | ||
|
||
describe('subGrids', function () { | ||
it('sets the .subGrid property', function () { | ||
fixture = fixtureSetup( | ||
'<div style="overflow: scroll; height: 100px;">' + | ||
'<span style="display: inline-block; height: 300px" id="x">x</span>' + | ||
'</div>' | ||
); | ||
var vOverflow = fixture.children[0]; | ||
assert.isUndefined(vOverflow._subGrid); | ||
createGrid(); | ||
assert.isDefined(vOverflow._subGrid); | ||
assert.notEqual(vOverflow._grid, vOverflow._subGrid); | ||
}); | ||
|
||
it('sets the ._grid of children as the subGrid', function () { | ||
fixture = fixtureSetup( | ||
'<div style="overflow: scroll; height: 100px;">' + | ||
'<span style="display: inline-block; height: 300px" id="x">x</span>' + | ||
'</div>' | ||
); | ||
createGrid(); | ||
var vOverflow = fixture.children[0]; | ||
var vSpan = vOverflow.children[0]; | ||
assert.equal(vOverflow._subGrid, vSpan._grid); | ||
}); | ||
|
||
it('does not add scrollable children to the root grid', function () { | ||
fixture = fixtureSetup( | ||
'<div style="overflow: scroll; height: 100px;">' + | ||
'<span style="display: inline-block; height: 300px" id="x">x</span>' + | ||
'</div>' | ||
); | ||
createGrid(); | ||
var vSpan = fixture.children[0].children[0]; | ||
var position = findPositions(fixture._grid, vSpan); | ||
assert.isEmpty(position); | ||
}); | ||
|
||
it('adds scrollable children to the subGrid', function () { | ||
fixture = fixtureSetup( | ||
'<div style="overflow: scroll; height: 100px;">' + | ||
'<span style="display: inline-block; height: 300px" id="x">x</span>' + | ||
'</div>' | ||
); | ||
createGrid(); | ||
var vOverflow = fixture.children[0]; | ||
var vSpan = vOverflow.children[0]; | ||
var position = findPositions(vOverflow._subGrid, vSpan); | ||
assert.deepEqual(position, [ | ||
{ x: 0, y: 0 }, | ||
{ x: 1, y: 0 } | ||
]); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.