fix(target-size): update to match new spacing requirements#4117
fix(target-size): update to match new spacing requirements#4117
Conversation
| return { x, y: closestY }; | ||
| } | ||
| } | ||
| return Math.hypot(pointA.x - pointB.x, pointA.y - pointB.y); |
There was a problem hiding this comment.
This function exists in IE11 (I've verified)
| height: baseRect.bottom - baseRect.top, | ||
| width: baseRect.right - baseRect.left | ||
| }; | ||
| return new window.DOMRect( |
There was a problem hiding this comment.
I thought it was weird that split rects works on DOMRects but then doesn't return one, which meant that doing splitRect(rect1, rect2).left would return undefined instead of the x value.
| `); | ||
| const nodeA = fixture.children[1]; | ||
| const nodeB = fixture.children[3]; | ||
| assert.closeTo(getOffset(nodeA, nodeB), 38, round); |
There was a problem hiding this comment.
Is the reason that this isn't exactly 40 because you're using buttons? This is why I used links when I wrote this originally, no min space, no padding, no border. I think you should use links so we get more consistent / predictable numbers.
There was a problem hiding this comment.
It's because the distance between the centers of both buttons gets subtracted by the circle radius (50 - 12).
WilcoFiers
left a comment
There was a problem hiding this comment.
Needs updated integration tests
Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>
Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>
…-core into target-size-update
Disable the target-size rule. This rule was added in Axe 4.7, but changed in 4.8 to allow for spacing around the element. See dequelabs/axe-core#4117 for more information on this change. Moodle 4.2 has an earlier version of Axe which does not have this rule. Moodle 4.4 has a later, unaffected, version of Axe. The simplest solution here is to simply disable the buggy rule.
This mostly coverts the target-offset check to use the updated spacing exception from the spec. However, going over this we did notice that we still might be off in determining the center of the bounding rect is. The code currently only looks at the largest target rect but we are thinking we need to take into account the bounding box for all target rects and center on that. Will leave that as a tech debt ticket for after 4.8 so we can get this merged in on time.
Another tech debt ticket that needs to be created is to update the target-size check to use the new
getTargetSizefunction.Closes: #3891