Skip to content

Implementing overflow prop in View component #14527

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

Closed
wants to merge 4 commits into from

Conversation

HariniMalothu17
Copy link
Contributor

@HariniMalothu17 HariniMalothu17 commented Apr 14, 2025

Description

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Why

Implement the overflow property for the fabric implementation of View.
This property was available in RNW Paper via ViewViewManager,NativeUIManager.

See https://reactnative.dev/docs/layout-props#overflow for details.

Resolves #13103

What

Implemented the overflow property for the fabric implementation of View.

Screenshots

Recording.2025-04-14.163816.mp4

Testing

If you added tests that prove your changes are effective or that your feature works, add a few sentences here detailing the added test scenarios.

Changelog

Yes

Added a brief summary of overflow to use in the release notes for the next release.

Microsoft Reviewers: Open in CodeFlow

@HariniMalothu17 HariniMalothu17 requested a review from a team as a code owner April 14, 2025 10:43
@HariniMalothu17 HariniMalothu17 marked this pull request as draft April 14, 2025 10:49
);

// Apply the clipping path to the visual
Visual().as<::Microsoft::ReactNative::Composition::Experimental::IVisualInterop>()->SetClippingPath(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already setting the clipping path when we have borderRadii, in ComponentView::updateClippingPath. -- I think that is done today to ensure that the background has a rounded corner too.

So I think the fix here will be more complex than this.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity) label Apr 14, 2025
@anupriya13 anupriya13 requested a review from Copilot April 17, 2025 15:09
Copilot

This comment was marked as outdated.

@microsoft-github-policy-service microsoft-github-policy-service bot added the no-recent-activity Issue/PR has gone stale and may be closed (label applied by bot) label Apr 24, 2025
@HariniMalothu17 HariniMalothu17 self-assigned this Jun 16, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot removed Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity) no-recent-activity Issue/PR has gone stale and may be closed (label applied by bot) labels Jun 16, 2025
@HariniMalothu17 HariniMalothu17 requested a review from Copilot June 18, 2025 13:20
Copilot

This comment was marked as outdated.

@HariniMalothu17 HariniMalothu17 requested a review from Copilot June 18, 2025 13:27
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request implements the overflow property for the fabric implementation of View. The changes add a new member to track whether children should be clipped, update the view’s props and layout functions to compute and apply the appropriate clipping path, and add a sample usage in the playground.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionViewComponentView.h Added a new boolean flag, m_shouldClipChildren, to track clipping behavior.
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionViewComponentView.cpp Updated updateProps to compute overflow and updated updateLayoutMetrics to apply or remove a clipping path based on that overflow.
packages/playground/Samples/view.tsx Added a sample view that uses the new overflow property.
change/react-native-windows-4a6abd7a-5513-40a7-bc6d-a7527518ba51.json Introduced a change log entry for the overflow property implementation.

@@ -1268,6 +1271,30 @@ void ViewComponentView::updateLayoutMetrics(
Visual().Size(
{layoutMetrics.frame.size.width * layoutMetrics.pointScaleFactor,
layoutMetrics.frame.size.height * layoutMetrics.pointScaleFactor});

if (m_shouldClipChildren) {
Copy link
Preview

Copilot AI Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider caching the computed clipping path when layout metrics and border metrics remain unchanged, to avoid redundant calculations on frequent layout updates.

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Currently updateClippingPath sets a clipping path if the view has any borderRadii. So with your change, if you were to set overflow = visible, then set a border radius, the code in updateClippingPath would set the border radius and ignore your overflow setting.

Also if you set overflow = visible, and have a borderradius, and have a backgroundColor, I think you'd end up with the background no longer having a borderradius.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement overflow property for View for fabric
2 participants