Tidied up; merged adjacent rects

packages/react-dom/src/__tests__/ReactDOMTestSelectors-test.internal.js

@@ -710,6 +710,105 @@ No matching component was found for:
       });
     });
 
+    it('should merge some types of adjacent rects (if they are the same in one dimension)', () => {
+      const refA = React.createRef();
+      const refB = React.createRef();
+      const refC = React.createRef();
+      const refD = React.createRef();
+      const refE = React.createRef();
+      const refF = React.createRef();
+      const refG = React.createRef();
+
+      function Example() {
+        return (
+          <>
+            <div ref={refA} data-debug="A" />
+            <div ref={refB} data-debug="B" />
+            <div ref={refC} data-debug="C" />
+            <div ref={refD} data-debug="D" />
+            <div ref={refE} data-debug="E" />
+            <div ref={refF} data-debug="F" />
+            <div ref={refG} data-debug="G" />
+          </>
+        );
+      }
+
+      render(<Example />, container);
+
+      // A, B, and C are all adjacent and/or overlapping, with the same height.
+      setBoundingClientRect(refA.current, {
+        x: 30,
+        y: 0,
+        width: 40,
+        height: 25,
+      });
+      setBoundingClientRect(refB.current, {
+        x: 0,
+        y: 0,
+        width: 50,
+        height: 25,
+      });
+      setBoundingClientRect(refC.current, {
+        x: 70,
+        y: 0,
+        width: 20,
+        height: 25,
+      });
+
+      // D is partially overlapping with A and B, but is too tall to be merged.
+      setBoundingClientRect(refD.current, {
+        x: 20,
+        y: 0,
+        width: 20,
+        height: 30,
+      });
+
+      // Same thing but for a vertical group.
+      // Some of them could intersect with the horizontal group,
+      // except they're too far to the right.
+      setBoundingClientRect(refE.current, {
+        x: 100,
+        y: 25,
+        width: 25,
+        height: 50,
+      });
+      setBoundingClientRect(refF.current, {
+        x: 100,
+        y: 0,
+        width: 25,
+        height: 25,
+      });
+      setBoundingClientRect(refG.current, {
+        x: 100,
+        y: 75,
+        width: 25,
+        height: 10,
+      });
+
+      const rects = findBoundingRects(document.body, [
+        createComponentSelector(Example),
+      ]);
+      expect(rects).toHaveLength(3);
+      expect(rects).toContainEqual({
+        x: 0,
+        y: 0,
+        width: 90,
+        height: 25,
+      });
+      expect(rects).toContainEqual({
+        x: 20,
+        y: 0,
+        width: 20,
+        height: 30,
+      });
+      expect(rects).toContainEqual({
+        x: 100,
+        y: 0,
+        width: 25,
+        height: 85,
+      });
+    });
+
     it('should not search within hidden subtrees', () => {
       const refA = React.createRef();
       const refB = React.createRef();

packages/react-dom/src/client/DOMAccessibilityRoles.js

@@ -131,7 +131,6 @@ function getExplicitRole(element: Element): string | null {
 // https://w3c.github.io/html-aria/#document-conformance-requirements-for-use-of-aria-attributes-in-html
 export function getRole(element: Element): string | null {
   const explicitRole = getExplicitRole(element);
-
   if (explicitRole !== null) {
     return explicitRole;
   }

packages/react-dom/src/client/ReactDOMHostConfig.js

@@ -1257,7 +1257,7 @@ export function validateEventListenerTarget(
 
 export const supportsTestSelectors = true;
 
-export function findRootFiber(node: Instance): null | FiberRoot {
+export function findFiberRoot(node: Instance): null | FiberRoot {
   const stack = [node];
   let index = 0;
   while (index < stack.length) {
@@ -1280,12 +1280,9 @@ export function getBoundingRect(node: Instance): BoundingRect {
   };
 }
 
-export function matchAccessibilityRole(fiber: Fiber, role: string): boolean {
-  if (fiber.tag === HostComponent) {
-    const node = fiber.stateNode;
-    if (role === getRole(node)) {
-      return true;
-    }
+export function matchAccessibilityRole(node: Instance, role: string): boolean {
+  if (role === getRole(node)) {
+    return true;
   }
 
   return false;
@@ -1310,8 +1307,8 @@ export function getTextContent(fiber: Fiber): string | null {
   return null;
 }
 
-export function isHiddenSubtree(workInProgress: Fiber): boolean {
-  return workInProgress.pendingProps.hidden === true;
+export function isHiddenSubtree(fiber: Fiber): boolean {
+  return fiber.tag === HostComponent && fiber.memoizedProps.hidden === true;
 }
 
 export function setFocusIfFocusable(node: Instance): boolean {

packages/react-reconciler/src/ReactFiberHostConfigWithNoTestSelectors.js

@@ -23,7 +23,7 @@ function shim(...args: any) {
 
 // Test selectors (when unsupported)
 export const supportsTestSelectors = false;
-export const findRootFiber = shim;
+export const findFiberRoot = shim;
 export const getBoundingRect = shim;
 export const getTextContent = shim;
 export const isHiddenSubtree = shim;

packages/react-reconciler/src/ReactTestSelectors.js

@@ -14,7 +14,7 @@ import invariant from 'shared/invariant';
 import {HostComponent, HostText} from 'react-reconciler/src/ReactWorkTags';
 import getComponentName from 'shared/getComponentName';
 import {
-  findRootFiber,
+  findFiberRoot,
   getBoundingRect,
   getInstanceFromNode,
   getTextContent,
@@ -113,7 +113,7 @@ export function createTestNameSelector(id: string): TestNameSelector {
   };
 }
 
-function findRootFiberForHostRoot(hostRoot: Instance): Fiber {
+function findFiberRootForHostRoot(hostRoot: Instance): Fiber {
   const maybeFiber = getInstanceFromNode((hostRoot: any));
   if (maybeFiber != null) {
     invariant(
@@ -122,7 +122,7 @@ function findRootFiberForHostRoot(hostRoot: Instance): Fiber {
     );
     return ((maybeFiber: any): Fiber);
   } else {
-    const fiberRoot = findRootFiber(hostRoot);
+    const fiberRoot = findFiberRoot(hostRoot);
     invariant(
       fiberRoot !== null,
       'Could not find React container within specified host subtree.',
@@ -147,8 +147,9 @@ function matchSelector(fiber: Fiber, selector: Selector): boolean {
       );
     case ROLE_TYPE:
       if (fiber.tag === HostComponent) {
+        const node = fiber.stateNode;
         if (
-          matchAccessibilityRole(fiber, ((selector: any): RoleSelector).value)
+          matchAccessibilityRole(node, ((selector: any): RoleSelector).value)
         ) {
           return true;
         }
@@ -178,7 +179,7 @@ function matchSelector(fiber: Fiber, selector: Selector): boolean {
       }
       break;
     default:
-      invariant(null, 'Invalid selector type %s specified', selector);
+      invariant(null, 'Invalid selector type %s specified.', selector);
       break;
   }
 
@@ -199,7 +200,7 @@ function selectorToString(selector: Selector): string | null {
     case TEST_NAME_TYPE:
       return `[data-testname="${((selector: any): TestNameSelector).value}"]`;
     default:
-      invariant(null, 'Invalid selector type %s specified', selector);
+      invariant(null, 'Invalid selector type %s specified.', selector);
       break;
   }
 
@@ -279,7 +280,7 @@ export function findAllNodes(
     invariant(false, 'Test selector API is not supported by this renderer.');
   }
 
-  const root = findRootFiberForHostRoot(hostRoot);
+  const root = findFiberRootForHostRoot(hostRoot);
   const matchingFibers = findPaths(root, selectors);
 
   const instanceRoots: Array<Instance> = [];
@@ -313,7 +314,7 @@ export function getFindAllNodesFailureDescription(
     invariant(false, 'Test selector API is not supported by this renderer.');
   }
 
-  const root = findRootFiberForHostRoot(hostRoot);
+  const root = findFiberRootForHostRoot(hostRoot);
 
   let maxSelectorIndex: number = 0;
   const matchedNames = [];
@@ -385,27 +386,78 @@ export function findBoundingRects(
     boundingRects.push(getBoundingRect(instanceRoots[i]));
   }
 
-  for (let i = boundingRects.length - 1; i >= 0; i--) {
+  for (let i = boundingRects.length - 1; i > 0; i--) {
     const targetRect = boundingRects[i];
-    for (let j = boundingRects.length - 1; j >= 0; j--) {
+    const targetLeft = targetRect.x;
+    const targetRight = targetLeft + targetRect.width;
+    const targetTop = targetRect.y;
+    const targetBottom = targetTop + targetRect.height;
+
+    for (let j = i - 1; j >= 0; j--) {
       if (i !== j) {
         const otherRect = boundingRects[j];
+        const otherLeft = otherRect.x;
+        const otherRight = otherLeft + otherRect.width;
+        const otherTop = otherRect.y;
+        const otherBottom = otherTop + otherRect.height;
+
+        // Merging all rects to the minimums set would be complicated,
+        // but we can handle the most common cases:
+        // 1. completely overlapping rects
+        // 2. adjacent rects that are the same width or height (e.g. items in a list)
+        //
+        // Even given the above constraints,
+        // we still won't end up with the fewest possible rects without doing multiple passes,
+        // but it's good enough for this purpose.
+
         if (
-          targetRect.x >= otherRect.x &&
-          targetRect.y >= otherRect.y &&
-          targetRect.x + targetRect.width <= otherRect.x + otherRect.width &&
-          targetRect.y + targetRect.height <= otherRect.y + otherRect.height
+          targetLeft >= otherLeft &&
+          targetTop >= otherTop &&
+          targetRight <= otherRight &&
+          targetBottom <= otherBottom
+        ) {
+          // Complete overlapping rects; remove the inner one.
+          boundingRects.splice(i, 1);
+          break;
+        } else if (
+          targetLeft === otherLeft &&
+          targetRect.width === otherRect.width &&
+          !(otherBottom < targetTop) &&
+          !(otherTop > targetBottom)
         ) {
+          // Adjacent vertical rects; merge them.
+          if (otherTop > targetTop) {
+            otherRect.height += otherTop - targetTop;
+            otherRect.y = targetTop;
+          }
+          if (otherBottom < targetBottom) {
+            otherRect.height = targetBottom - otherTop;
+          }
+
+          boundingRects.splice(i, 1);
+          break;
+        } else if (
+          targetTop === otherTop &&
+          targetRect.height === otherRect.height &&
+          !(otherRight < targetLeft) &&
+          !(otherLeft > targetRight)
+        ) {
+          // Adjacent horizontal rects; merge them.
+          if (otherLeft > targetLeft) {
+            otherRect.width += otherLeft - targetLeft;
+            otherRect.x = targetLeft;
+          }
+          if (otherRight < targetRight) {
+            otherRect.width = targetRight - otherLeft;
+          }
+
           boundingRects.splice(i, 1);
           break;
         }
       }
     }
   }
 
-  // TODO We may also want to combine rects that intersect or are adjacent.
-  // We could at least handle the most common cases (e.g. same in one dimension).
-
   return boundingRects;
 }
 
@@ -417,23 +469,23 @@ export function focusWithin(
     invariant(false, 'Test selector API is not supported by this renderer.');
   }
 
-  const root = findRootFiberForHostRoot(hostRoot);
+  const root = findFiberRootForHostRoot(hostRoot);
   const matchingFibers = findPaths(root, selectors);
 
   const stack = Array.from(matchingFibers);
   let index = 0;
   while (index < stack.length) {
-    const node = ((stack[index++]: any): Fiber);
-    if (node.tag === HostComponent) {
-      if (isHiddenSubtree(node)) {
-        continue;
-      }
-      if (setFocusIfFocusable(node.stateNode)) {
+    const fiber = ((stack[index++]: any): Fiber);
+    if (isHiddenSubtree(fiber)) {
+      continue;
+    }
+    if (fiber.tag === HostComponent) {
+      const node = fiber.stateNode;
+      if (setFocusIfFocusable(node)) {
         return true;
       }
     }
-
-    let child = node.child;
+    let child = fiber.child;
     while (child !== null) {
       stack.push(child);
       child = child.sibling;

packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js

@@ -95,7 +95,7 @@ export const makeServerId = $$$hostConfig.makeServerId;
 //     (optional)
 // -------------------
 export const supportsTestSelectors = $$$hostConfig.supportsTestSelectors;
-export const findRootFiber = $$$hostConfig.findRootFiber;
+export const findFiberRoot = $$$hostConfig.findFiberRoot;
 export const getBoundingRect = $$$hostConfig.getBoundingRect;
 export const getTextContent = $$$hostConfig.getTextContent;
 export const isHiddenSubtree = $$$hostConfig.isHiddenSubtree;

scripts/error-codes/codes.json

@@ -354,10 +354,8 @@
   "354": "getInspectorDataForViewAtPoint() is not available in production.",
   "355": "The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. Do not read the value directly.",
   "356": "The current renderer does not support test selectors. This error is likely caused by a bug in React. Please file an issue.",
-  "357": "findAllNodes() is not supported by this renderer.",
-  "358": "Could not find React container within specified host subtree.",
-  "359": "The current renderer does not support test selectors. This error is likely caused by a bug in React. Please file an issue.",
-  "360": "Test selector API is not supported by this renderer.",
-  "361": "Invalid host root specified. Should be either a React container or a node with a testname attribute.",
-  "362": "Invalid selector type %s specified"
+  "357": "Could not find React container within specified host subtree.",
+  "358": "Test selector API is not supported by this renderer.",
+  "359": "Invalid host root specified. Should be either a React container or a node with a testname attribute.",
+  "360": "Invalid selector type %s specified."
 }