Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make getContentOriginOffset to know info about if call-site want transform or not #44822

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,11 @@ void ScrollViewShadowNode::layout(LayoutContext layoutContext) {
updateStateIfNeeded();
}

Point ScrollViewShadowNode::getContentOriginOffset() const {
Point ScrollViewShadowNode::getContentOriginOffset(
bool /*includeTransform*/) const {
auto stateData = getStateData();
auto contentOffset = stateData.contentOffset;

return {-contentOffset.x, -contentOffset.y + stateData.scrollAwayPaddingTop};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class ScrollViewShadowNode final : public ConcreteViewShadowNode<
#pragma mark - LayoutableShadowNode

void layout(LayoutContext layoutContext) override;
Point getContentOriginOffset() const override;
Point getContentOriginOffset(bool includeTransform) const override;

private:
void updateStateIfNeeded();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,11 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics(
}

if (i != 0 && policy.includeTransform) {
resultFrame.origin += currentShadowNode->getContentOriginOffset();
// Transformation is not applied here and instead we delegated out in
// getContentOriginOffset. The reason is that for `ScrollViewShadowNode`,
// we need to consider `scrollAwayPaddingTop` which should NOT be included
// in the transform.
resultFrame.origin += currentShadowNode->getContentOriginOffset(true);
}

if (policy.enableOverflowClipping) {
Expand Down Expand Up @@ -188,7 +192,8 @@ Transform LayoutableShadowNode::getTransform() const {
return Transform::Identity();
}

Point LayoutableShadowNode::getContentOriginOffset() const {
Point LayoutableShadowNode::getContentOriginOffset(
bool /*includeTransform*/) const {
return {0, 0};
}

Expand Down Expand Up @@ -269,7 +274,7 @@ ShadowNode::Shared LayoutableShadowNode::findNodeAtPoint(
}

auto newPoint = point - transformedFrame.origin -
layoutableShadowNode->getContentOriginOffset();
layoutableShadowNode->getContentOriginOffset(false);

auto sortedChildren = node->getChildren();
std::stable_sort(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,11 @@ class LayoutableShadowNode : public ShadowNode {
* Returns offset which is applied to children's origin in
* `LayoutableShadowNode::getRelativeLayoutMetrics` and
* `LayoutableShadowNode::findNodeAtPoint`.
* i`ncludeTransform` is a flag to include the transform in the offset. This
* is a rare case but needed for case where transform is involved for e.g. in
* ScrollView.
*/
virtual Point getContentOriginOffset() const;
virtual Point getContentOriginOffset(bool includeTransform) const;

/*
* Sets layout metrics for the shadow node.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ class TestShadowNode final : public ConcreteViewShadowNode<

facebook::react::Point _contentOriginOffset{};

facebook::react::Point getContentOriginOffset() const override {
facebook::react::Point getContentOriginOffset(
bool /*includeTransform*/) const override {
return _contentOriginOffset;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ DOMPoint getScrollPosition(
return DOMPoint{};
}

auto scrollPosition = layoutableShadowNode->getContentOriginOffset();
auto scrollPosition = layoutableShadowNode->getContentOriginOffset(false);

return DOMPoint{
.x = scrollPosition.x == 0 ? 0 : -scrollPosition.x,
Expand Down
Loading