fix(target-size): update to match new spacing requirements (#4117)

* fix(target-size): update to match new spacing requirements * working * tests * finalize? * tests * revert playground * 🤖 Automated formatting fixes * Apply suggestions from code review Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com> * 🤖 Automated formatting fixes * udpate tests * dont half minOffset but double return from getOffset * Apply suggestions from code review Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com> * 🤖 Automated formatting fixes * fix tests * fix test --------- Co-authored-by: straker <straker@users.noreply.github.com> Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>
dequelabs · Aug 8, 2023 · 49eaa0e · 49eaa0e
1 parent fcf76e0
commit 49eaa0e
Show file tree

Hide file tree

Showing 12 changed files with 359 additions and 346 deletions.
diff --git a/lib/checks/mobile/target-offset-evaluate.js b/lib/checks/mobile/target-offset-evaluate.js
@@ -12,7 +12,9 @@ export default function targetOffsetEvaluate(node, options, vNode) {
     if (getRoleType(vNeighbor) !== 'widget' || !isFocusable(vNeighbor)) {
       continue;
     }
-    const offset = roundToSingleDecimal(getOffset(vNode, vNeighbor));
+    // the offset code works off radius but we want our messaging to reflect diameter
+    const offset =
+      roundToSingleDecimal(getOffset(vNode, vNeighbor, minOffset / 2)) * 2;
     if (offset + roundingMargin >= minOffset) {
       continue;
     }

diff --git a/lib/checks/mobile/target-offset.json b/lib/checks/mobile/target-offset.json
@@ -7,11 +7,11 @@
   "metadata": {
     "impact": "serious",
     "messages": {
-      "pass": "Target has sufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)",
-      "fail": "Target has insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)",
+      "pass": "Target has sufficient space from its closest neighbors (${data.closestOffset}px should be at least ${data.minOffset}px)",
+      "fail": "Target has insufficient space to its closest neighbors. Safe clickable space has a diameter of {$data.closestOffset}px instead of at least ${data.minOffset}px)",
       "incomplete": {
-        "default": "Element with negative tabindex has insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px). Is this a target?",
-        "nonTabbableNeighbor": "Target has insufficient offset from a neighbor with negative tabindex (${data.closestOffset}px should be at least ${data.minOffset}px). Is the neighbor a target?"
+        "default": "Element with negative tabindex has insufficient space to its closest neighbors. Safe clickable space has a diameter of {$data.closestOffset}px instead of at least ${data.minOffset}px). Is this a target?",
+        "nonTabbableNeighbor": "Target has insufficient space to its closest neighbors. Safe clickable space has a diameter of {$data.closestOffset}px instead of at least ${data.minOffset}px). Is the neighbor a target?"
       }
     }
   }

diff --git a/lib/commons/dom/get-target-size.js b/lib/commons/dom/get-target-size.js
@@ -0,0 +1,66 @@
+import findNearbyElms from './find-nearby-elms';
+import { splitRects, hasVisualOverlap } from '../math';
+import memoize from '../../core/utils/memoize';
+
+const roundingMargin = 0.05;
+
+export default memoize(getTargetSize);
+
+/**
+ * Compute the target size of an element.
+ * @see https://www.w3.org/TR/WCAG22/#dfn-targets
+ */
+function getTargetSize(vNode, minSize) {
+  const nodeRect = vNode.boundingClientRect;
+  const overlappingVNodes = findNearbyElms(vNode).filter(vNeighbor => {
+    return (
+      vNeighbor.getComputedStylePropertyValue('pointer-events') !== 'none' &&
+      hasVisualOverlap(vNode, vNeighbor)
+    );
+  });
+
+  if (!overlappingVNodes.length) {
+    return nodeRect;
+  }
+
+  return getLargestUnobscuredArea(vNode, overlappingVNodes, minSize);
+}
+
+// Find areas of the target that are not obscured
+function getLargestUnobscuredArea(vNode, obscuredNodes, minSize) {
+  const nodeRect = vNode.boundingClientRect;
+  if (obscuredNodes.length === 0) {
+    return null;
+  }
+  const obscuringRects = obscuredNodes.map(
+    ({ boundingClientRect: rect }) => rect
+  );
+  const unobscuredRects = splitRects(nodeRect, obscuringRects);
+  if (!unobscuredRects.length) {
+    return null;
+  }
+
+  // Of the unobscured inner rects, work out the largest
+  return getLargestRect(unobscuredRects, minSize);
+}
+
+// Find the largest rectangle in the array, prioritize ones that meet a minimum size
+function getLargestRect(rects, minSize) {
+  return rects.reduce((rectA, rectB) => {
+    const rectAisMinimum = rectHasMinimumSize(minSize, rectA);
+    const rectBisMinimum = rectHasMinimumSize(minSize, rectB);
+    // Prioritize rects that pass the minimum
+    if (rectAisMinimum !== rectBisMinimum) {
+      return rectAisMinimum ? rectA : rectB;
+    }
+    const areaA = rectA.width * rectA.height;
+    const areaB = rectB.width * rectB.height;
+    return areaA > areaB ? rectA : rectB;
+  });
+}
+
+function rectHasMinimumSize(minSize, { width, height }) {
+  return (
+    width + roundingMargin >= minSize && height + roundingMargin >= minSize
+  );
+}
diff --git a/lib/commons/dom/index.js b/lib/commons/dom/index.js
@@ -17,6 +17,7 @@ export { default as getOverflowHiddenAncestors } from './get-overflow-hidden-anc
 export { default as getRootNode } from './get-root-node';
 export { default as getScrollOffset } from './get-scroll-offset';
 export { default as getTabbableElements } from './get-tabbable-elements';
+export { default as getTargetSize } from './get-target-size';
 export { default as getTextElementStack } from './get-text-element-stack';
 export { default as getViewportSize } from './get-viewport-size';
 export { default as getVisibleChildTextRects } from './get-visible-child-text-rects';

diff --git a/lib/commons/math/get-offset.js b/lib/commons/math/get-offset.js
@@ -1,91 +1,61 @@
+import { getTargetSize } from '../dom';
+
 /**
  * Get the offset between node A and node B
  * @method getOffset
  * @memberof axe.commons.math
  * @param {VirtualNode} vNodeA
  * @param {VirtualNode} vNodeB
+ * @param {Number} radius
  * @returns {number}
  */
-export default function getOffset(vNodeA, vNodeB) {
-  const rectA = vNodeA.boundingClientRect;
-  const rectB = vNodeB.boundingClientRect;
-  const pointA = getFarthestPoint(rectA, rectB);
-  const pointB = getClosestPoint(pointA, rectA, rectB);
-  return pointDistance(pointA, pointB);
-}
-
-/**
- * Get a point on rectA that is farthest away from rectB
- * @param {Rect} rectA
- * @param {Rect} rectB
- * @returns {Point}
- */
-function getFarthestPoint(rectA, rectB) {
-  const dimensionProps = [
-    ['x', 'left', 'right', 'width'],
-    ['y', 'top', 'bottom', 'height']
-  ];
-  const farthestPoint = {};
-  dimensionProps.forEach(([axis, start, end, diameter]) => {
-    if (rectB[start] < rectA[start] && rectB[end] > rectA[end]) {
-      farthestPoint[axis] = rectA[start] + rectA[diameter] / 2; // center | middle
-      return;
-    }
-    // Work out which edge of A is farthest away from the center of B
-    const centerB = rectB[start] + rectB[diameter] / 2;
-    const startDistance = Math.abs(centerB - rectA[start]);
-    const endDistance = Math.abs(centerB - rectA[end]);
-    if (startDistance >= endDistance) {
-      farthestPoint[axis] = rectA[start]; // left | top
-    } else {
-      farthestPoint[axis] = rectA[end]; // right | bottom
-    }
-  });
-  return farthestPoint;
-}
+export default function getOffset(vNodeA, vNodeB, minRadiusNeighbour = 12) {
+  const rectA = getTargetSize(vNodeA);
+  const rectB = getTargetSize(vNodeB);
 
-/**
- * Get a point on the adjacentRect, that is as close the point given from ownRect
- * @param {Point} ownRectPoint
- * @param {Rect} ownRect
- * @param {Rect} adjacentRect
- * @returns {Point}
- */
-function getClosestPoint({ x, y }, ownRect, adjacentRect) {
-  if (pointInRect({ x, y }, adjacentRect)) {
-    // Check if there is an opposite corner inside the adjacent rectangle
-    const closestPoint = getCornerInAdjacentRect(
-      { x, y },
-      ownRect,
-      adjacentRect
-    );
-    if (closestPoint !== null) {
-      return closestPoint;
-    }
-    adjacentRect = ownRect;
+  // one of the rects is fully obscured
+  if (rectA === null || rectB === null) {
+    return 0;
   }
 
-  const { top, right, bottom, left } = adjacentRect;
-  // Is the adjacent rect horizontally or vertically aligned
-  const xAligned = x >= left && x <= right;
-  const yAligned = y >= top && y <= bottom;
-  // Find the closest edge of the adjacent rect
-  const closestX = Math.abs(left - x) < Math.abs(right - x) ? left : right;
-  const closestY = Math.abs(top - y) < Math.abs(bottom - y) ? top : bottom;
+  const centerA = {
+    x: rectA.x + rectA.width / 2,
+    y: rectA.y + rectA.height / 2
+  };
+  const centerB = {
+    x: rectB.x + rectB.width / 2,
+    y: rectB.y + rectB.height / 2
+  };
+  const sideB = getClosestPoint(centerA, rectB);
 
-  if (!xAligned && yAligned) {
-    return { x: closestX, y }; // Closest horizontal point
-  } else if (xAligned && !yAligned) {
-    return { x, y: closestY }; // Closest vertical point
-  } else if (!xAligned && !yAligned) {
-    return { x: closestX, y: closestY }; // Closest diagonal corner
+  return Math.min(
+    // subtract the radius of the circle from the distance
+    pointDistance(centerA, centerB) - minRadiusNeighbour,
+    pointDistance(centerA, sideB)
+  );
+}
+
+function getClosestPoint(point, rect) {
+  let x;
+  let y;
+
+  if (point.x < rect.left) {
+    x = rect.left;
+  } else if (point.x > rect.right) {
+    x = rect.right;
+  } else {
+    x = point.x;
   }
-  // ownRect (partially) obscures adjacentRect
-  if (Math.abs(x - closestX) < Math.abs(y - closestY)) {
-    return { x: closestX, y }; // Inside, closest edge is horizontal
+
+  if (point.y < rect.top) {
+    y = rect.top;
+  } else if (point.y > rect.bottom) {
+    y = rect.bottom;
   } else {
-    return { x, y: closestY }; // Inside, closest edge is vertical
+    y = point.y;
   }
+
+  return { x, y };
 }
 
 /**
@@ -95,55 +65,5 @@ function getClosestPoint({ x, y }, ownRect, adjacentRect) {
  * @returns {number}
  */
 function pointDistance(pointA, pointB) {
-  const xDistance = Math.abs(pointA.x - pointB.x);
-  const yDistance = Math.abs(pointA.y - pointB.y);
-  if (!xDistance || !yDistance) {
-    return xDistance || yDistance; // If either is 0, return the other
-  }
-  return Math.sqrt(Math.pow(xDistance, 2) + Math.pow(yDistance, 2));
-}
-
-/**
- * Return if a point is within a rect
- * @param {Point} point
- * @param {Rect} rect
- * @returns {boolean}
- */
-function pointInRect({ x, y }, rect) {
-  return y >= rect.top && x <= rect.right && y <= rect.bottom && x >= rect.left;
-}
-
-/**
- *
- * @param {Point} ownRectPoint
- * @param {Rect} ownRect
- * @param {Rect} adjacentRect
- * @returns {Point | null} With x and y
- */
-function getCornerInAdjacentRect({ x, y }, ownRect, adjacentRect) {
-  let closestX, closestY;
-  // Find the opposite corner, if it is inside the adjacent rect;
-  if (x === ownRect.left && ownRect.right < adjacentRect.right) {
-    closestX = ownRect.right;
-  } else if (x === ownRect.right && ownRect.left > adjacentRect.left) {
-    closestX = ownRect.left;
-  }
-  if (y === ownRect.top && ownRect.bottom < adjacentRect.bottom) {
-    closestY = ownRect.bottom;
-  } else if (y === ownRect.bottom && ownRect.top > adjacentRect.top) {
-    closestY = ownRect.top;
-  }
-
-  if (!closestX && !closestY) {
-    return null; // opposite corners are outside the rect, or {x,y} was a center point
-  } else if (!closestY) {
-    return { x: closestX, y };
-  } else if (!closestX) {
-    return { x, y: closestY };
-  }
-  if (Math.abs(x - closestX) < Math.abs(y - closestY)) {
-    return { x: closestX, y };
-  } else {
-    return { x, y: closestY };
-  }
+  return Math.hypot(pointA.x - pointB.x, pointA.y - pointB.y);
 }
diff --git a/lib/commons/math/split-rects.js b/lib/commons/math/split-rects.js
@@ -5,7 +5,7 @@
  * @memberof axe.commons.math
  * @param {DOMRect} outerRect
  * @param {DOMRect[]} overlapRects
- * @returns {Rect[]} Unique array of rects
+ * @returns {DOMRect[]} Unique array of rects
  */
 export default function splitRects(outerRect, overlapRects) {
   let uniqueRects = [outerRect];
@@ -37,19 +37,33 @@ function splitRect(inputRect, clipRect) {
     rects.push({ top, left, bottom, right: clipRect.left });
   }
   if (rects.length === 0) {
+    // Fully overlapping
+    if (isEnclosedRect(inputRect, clipRect)) {
+      return [];
+    }
+
     rects.push(inputRect); // No intersection
   }
+
   return rects.map(computeRect); // add x / y / width / height
 }
 
 const between = (num, min, max) => num > min && num < max;
 
 function computeRect(baseRect) {
-  return {
-    ...baseRect,
-    x: baseRect.left,
-    y: baseRect.top,
-    height: baseRect.bottom - baseRect.top,
-    width: baseRect.right - baseRect.left
-  };
+  return new window.DOMRect(
+    baseRect.left,
+    baseRect.top,
+    baseRect.right - baseRect.left,
+    baseRect.bottom - baseRect.top
+  );
+}
+
+function isEnclosedRect(rectA, rectB) {
+  return (
+    rectA.top >= rectB.top &&
+    rectA.left >= rectB.left &&
+    rectA.bottom <= rectB.bottom &&
+    rectA.right <= rectB.right
+  );
 }
diff --git a/locales/_template.json b/locales/_template.json
@@ -862,11 +862,11 @@
       "fail": "${data} on <meta> tag disables zooming on mobile devices"
     },
     "target-offset": {
-      "pass": "Target has sufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)",
-      "fail": "Target has insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)",
+      "pass": "Target has sufficient space from its closest neighbors (${data.closestOffset}px should be at least ${data.minOffset}px)",
+      "fail": "Target has insufficient space to its closest neighbors. Safe clickable space has a diameter of {$data.closestOffset}px instead of at least ${data.minOffset}px)",
       "incomplete": {
-        "default": "Element with negative tabindex has insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px). Is this a target?",
-        "nonTabbableNeighbor": "Target has insufficient offset from a neighbor with negative tabindex (${data.closestOffset}px should be at least ${data.minOffset}px). Is the neighbor a target?"
+        "default": "Element with negative tabindex has insufficient space to its closest neighbors. Safe clickable space has a diameter of {$data.closestOffset}px instead of at least ${data.minOffset}px). Is this a target?",
+        "nonTabbableNeighbor": "Target has insufficient space to its closest neighbors. Safe clickable space has a diameter of {$data.closestOffset}px instead of at least ${data.minOffset}px). Is the neighbor a target?"
       }
     },
     "target-size": {