Skip to content

Commit

Permalink
Consider transforms correctly in some of the new DOM layout methods (#…
Browse files Browse the repository at this point in the history
…39349)

Summary:
Pull Request resolved: #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
  • Loading branch information
rubennorte authored and facebook-github-bot committed Sep 8, 2023
1 parent 836511e commit 016e4af
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 24 deletions.
10 changes: 7 additions & 3 deletions packages/react-native/Libraries/DOM/Nodes/ReactNativeElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
);
}

/**
Expand Down
41 changes: 27 additions & 14 deletions packages/react-native/Libraries/DOM/Nodes/ReadOnlyElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -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});
}

/**
Expand Down Expand Up @@ -262,3 +249,29 @@ function getChildElements(node: ReadOnlyNode): $ReadOnlyArray<ReadOnlyElement> {
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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export interface Spec {
+getTextContent: (node: Node) => string;
+getBoundingClientRect: (
node: Node,
includeTransform: boolean,
) => ?[
/* x: */ number,
/* y: */ number,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ const FabricUIManagerMock: IFabricUIManagerMock = {
getBoundingClientRect: jest.fn(
(
node: Node,
includeTransform: boolean,
): ?[
/* x:*/ number,
/* y:*/ number,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 016e4af

Please sign in to comment.