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
Summary:
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. I'll do it in a follow-up diff.

Changelog: [internal]

Reviewed By: NickGerleman

Differential Revision: D49069517

fbshipit-source-id: 1b26be6517f0bd1516c8eb5c2d2fdf1021257bf7
  • Loading branch information
rubennorte authored and facebook-github-bot committed Sep 8, 2023
1 parent 7d49d05 commit c38bc62
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 c38bc62

Please sign in to comment.