From 016e4af11dea9fa4a1502360bf08268b9c8f6c6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Fri, 8 Sep 2023 05:34:34 -0700 Subject: [PATCH] Consider transforms correctly in some of the new DOM layout methods (#39349) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/39349 This fixes these methods to ignore transforms, as per the spec: * `offsetLeft` * `offsetTop` * `offsetWidth` * `offsetHeight` * `clientLeft` * `clientTop` * `clientWidth` * `clientHeight` `scrollWidth` and `scrollHeight` are the last methods we need to fix, as their fix is more complex than in these cases (in scroll views, the scrollable area is the overflow of all its children with transforms applied, which is an expensive computation we don't currently do, even in host platforms where this behavior doesn't work correctly). Changelog: [internal] Reviewed By: NickGerleman Differential Revision: D49069517 fbshipit-source-id: 3c4b897c904e33514cbeefa8ee317d3c2e4a1280 --- .../Libraries/DOM/Nodes/ReactNativeElement.js | 10 +++-- .../Libraries/DOM/Nodes/ReadOnlyElement.js | 41 ++++++++++++------- .../Libraries/ReactNative/FabricUIManager.js | 1 + .../ReactFabricHostComponent.js | 2 +- .../ReactNative/__mocks__/FabricUIManager.js | 1 + .../renderer/uimanager/UIManagerBinding.cpp | 20 ++++++--- 6 files changed, 51 insertions(+), 24 deletions(-) diff --git a/packages/react-native/Libraries/DOM/Nodes/ReactNativeElement.js b/packages/react-native/Libraries/DOM/Nodes/ReactNativeElement.js index 491f8d10192..2900fe769f7 100644 --- a/packages/react-native/Libraries/DOM/Nodes/ReactNativeElement.js +++ b/packages/react-native/Libraries/DOM/Nodes/ReactNativeElement.js @@ -25,7 +25,7 @@ import TextInputState from '../../Components/TextInput/TextInputState'; import {getFabricUIManager} from '../../ReactNative/FabricUIManager'; import {create as createAttributePayload} from '../../ReactNative/ReactFabricPublicInstance/ReactNativeAttributePayload'; import warnForStyleProps from '../../ReactNative/ReactFabricPublicInstance/warnForStyleProps'; -import ReadOnlyElement from './ReadOnlyElement'; +import ReadOnlyElement, {getBoundingClientRect} from './ReadOnlyElement'; import ReadOnlyNode from './ReadOnlyNode'; import { getPublicInstanceFromInternalInstanceHandle, @@ -58,7 +58,9 @@ export default class ReactNativeElement } get offsetHeight(): number { - return Math.round(this.getBoundingClientRect().height); + return Math.round( + getBoundingClientRect(this, {includeTransform: false}).height, + ); } get offsetLeft(): number { @@ -110,7 +112,9 @@ export default class ReactNativeElement } get offsetWidth(): number { - return Math.round(this.getBoundingClientRect().width); + return Math.round( + getBoundingClientRect(this, {includeTransform: false}).width, + ); } /** diff --git a/packages/react-native/Libraries/DOM/Nodes/ReadOnlyElement.js b/packages/react-native/Libraries/DOM/Nodes/ReadOnlyElement.js index d81e8b8c6ad..03cf37949b3 100644 --- a/packages/react-native/Libraries/DOM/Nodes/ReadOnlyElement.js +++ b/packages/react-native/Libraries/DOM/Nodes/ReadOnlyElement.js @@ -211,20 +211,7 @@ export default class ReadOnlyElement extends ReadOnlyNode { } getBoundingClientRect(): DOMRect { - const shadowNode = getShadowNode(this); - - if (shadowNode != null) { - const rect = nullthrows(getFabricUIManager()).getBoundingClientRect( - shadowNode, - ); - - if (rect) { - return new DOMRect(rect[0], rect[1], rect[2], rect[3]); - } - } - - // Empty rect if any of the above failed - return new DOMRect(0, 0, 0, 0); + return getBoundingClientRect(this, {includeTransform: true}); } /** @@ -262,3 +249,29 @@ function getChildElements(node: ReadOnlyNode): $ReadOnlyArray { childNode => childNode instanceof ReadOnlyElement, ); } + +/** + * The public API for `getBoundingClientRect` always includes transform, + * so we use this internal version to get the data without transform to + * implement methods like `offsetWidth` and `offsetHeight`. + */ +export function getBoundingClientRect( + node: ReadOnlyElement, + {includeTransform}: {includeTransform: boolean}, +): DOMRect { + const shadowNode = getShadowNode(node); + + if (shadowNode != null) { + const rect = nullthrows(getFabricUIManager()).getBoundingClientRect( + shadowNode, + includeTransform, + ); + + if (rect) { + return new DOMRect(rect[0], rect[1], rect[2], rect[3]); + } + } + + // Empty rect if any of the above failed + return new DOMRect(0, 0, 0, 0); +} diff --git a/packages/react-native/Libraries/ReactNative/FabricUIManager.js b/packages/react-native/Libraries/ReactNative/FabricUIManager.js index 0c31dc97d61..99c8c438d49 100644 --- a/packages/react-native/Libraries/ReactNative/FabricUIManager.js +++ b/packages/react-native/Libraries/ReactNative/FabricUIManager.js @@ -75,6 +75,7 @@ export interface Spec { +getTextContent: (node: Node) => string; +getBoundingClientRect: ( node: Node, + includeTransform: boolean, ) => ?[ /* x: */ number, /* y: */ number, diff --git a/packages/react-native/Libraries/ReactNative/ReactFabricPublicInstance/ReactFabricHostComponent.js b/packages/react-native/Libraries/ReactNative/ReactFabricPublicInstance/ReactFabricHostComponent.js index f71d6fa9bf7..7ddad94e8ec 100644 --- a/packages/react-native/Libraries/ReactNative/ReactFabricPublicInstance/ReactFabricHostComponent.js +++ b/packages/react-native/Libraries/ReactNative/ReactFabricPublicInstance/ReactFabricHostComponent.js @@ -124,7 +124,7 @@ export default class ReactFabricHostComponent implements INativeMethods { this.__internalInstanceHandle, ); if (node != null) { - const rect = fabricGetBoundingClientRect(node); + const rect = fabricGetBoundingClientRect(node, true); if (rect) { return new DOMRect(rect[0], rect[1], rect[2], rect[3]); diff --git a/packages/react-native/Libraries/ReactNative/__mocks__/FabricUIManager.js b/packages/react-native/Libraries/ReactNative/__mocks__/FabricUIManager.js index 5a7442c5f50..83727b34537 100644 --- a/packages/react-native/Libraries/ReactNative/__mocks__/FabricUIManager.js +++ b/packages/react-native/Libraries/ReactNative/__mocks__/FabricUIManager.js @@ -295,6 +295,7 @@ const FabricUIManagerMock: IFabricUIManagerMock = { getBoundingClientRect: jest.fn( ( node: Node, + includeTransform: boolean, ): ?[ /* x:*/ number, /* y:*/ number, diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp index 98ba8c901ad..c00411b6c58 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp @@ -841,14 +841,20 @@ jsi::Value UIManagerBinding::get( // This is similar to `measureInWindow`, except it's explicitly synchronous // (returns the result instead of passing it to a callback). - // getBoundingClientRect(shadowNode: ShadowNode): + // It allows indicating whether to include transforms so it can also be used + // to implement methods like + // [`offsetWidth`](https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/offsetWidth) + // and + // [`offsetHeight`](https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/offsetHeight). + + // getBoundingClientRect(shadowNode: ShadowNode, includeTransform: boolean): // [ // /* x: */ number, // /* y: */ number, // /* width: */ number, // /* height: */ number // ] - auto paramCount = 1; + auto paramCount = 2; return jsi::Function::createFromHostFunction( runtime, name, @@ -860,10 +866,12 @@ jsi::Value UIManagerBinding::get( size_t count) -> jsi::Value { validateArgumentCount(runtime, methodName, paramCount, count); + bool includeTransform = arguments[1].getBool(); + auto layoutMetrics = uiManager->getRelativeLayoutMetrics( *shadowNodeFromValue(runtime, arguments[0]), nullptr, - {/* .includeTransform = */ true, + {/* .includeTransform = */ includeTransform, /* .includeViewportOffset = */ true}); if (layoutMetrics == EmptyLayoutMetrics) { @@ -1101,7 +1109,7 @@ jsi::Value UIManagerBinding::get( // If the node is not displayed (itself or any of its ancestors has // "display: none"), this returns an empty layout metrics object. auto layoutMetrics = uiManager->getRelativeLayoutMetrics( - *shadowNode, nullptr, {/* .includeTransform = */ true}); + *shadowNode, nullptr, {/* .includeTransform = */ false}); if (layoutMetrics == EmptyLayoutMetrics) { return jsi::Value::undefined(); @@ -1313,7 +1321,7 @@ jsi::Value UIManagerBinding::get( // If the node is not displayed (itself or any of its ancestors has // "display: none"), this returns an empty layout metrics object. auto layoutMetrics = uiManager->getRelativeLayoutMetrics( - *shadowNode, nullptr, {/* .includeTransform = */ true}); + *shadowNode, nullptr, {/* .includeTransform = */ false}); if (layoutMetrics == EmptyLayoutMetrics || layoutMetrics.displayType == DisplayType::Inline) { @@ -1367,7 +1375,7 @@ jsi::Value UIManagerBinding::get( // If the node is not displayed (itself or any of its ancestors has // "display: none"), this returns an empty layout metrics object. auto layoutMetrics = uiManager->getRelativeLayoutMetrics( - *shadowNode, nullptr, {/* .includeTransform = */ true}); + *shadowNode, nullptr, {/* .includeTransform = */ false}); if (layoutMetrics == EmptyLayoutMetrics || layoutMetrics.displayType == DisplayType::Inline) {