Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 8, 2025

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Fixes #31083

Summary

This PR refactors the ComputeConstraintForView method pattern across the MAUI layout system to eliminate side effects and improve code maintainability. Previously, ComputeConstraintForView was a void method that directly modified the child view's ComputedConstraint property. This change converts it to return a LayoutConstraint value instead, with the parent VisualElement responsible for assigning the returned value.

Key Changes

1. VisualElement Base Implementation

  • Changed ComputeConstraintForView signature from protected virtual void to protected virtual LayoutConstraint
  • Made ComputedConstraint setter internal (not private) to allow specific use cases like ViewCell
  • Updated constraint computation flows to use return values:
    • ComputeConstrainsForChildren(): child.ComputedConstraint = ComputeConstraintForView(child)
    • OnChildAdded(): view.ComputedConstraint = ComputeConstraintForView(view)
    • InvalidateMeasureInternal(): thisView.ComputedConstraint = visualParent.ComputeConstraintForView(thisView)

2. Layout Override Updates

Updated all 11 layout classes to return constraints instead of setting them:

  • ContentPresenter
  • StackLayout, VerticalStackLayout, HorizontalStackLayout
  • Grid (both modern and legacy versions)
  • AbsoluteLayout
  • TemplatedPage, TemplatedView
  • ScrollView

3. Special Cases

Maintained direct constraint assignments where needed:

  • ViewCell: Preserves constraint assignments in View property setter for cell layout behavior

Implementation Pattern

Before (side effects):

protected override void ComputeConstraintForView(View view)
{
    var result = LayoutConstraint.None;
    if (SomeCondition)
        result |= LayoutConstraint.HorizontallyFixed;
    
    view.ComputedConstraint = result; // Direct modification
}

After (return values):

protected override LayoutConstraint ComputeConstraintForView(View view)
{
    var result = LayoutConstraint.None;
    if (SomeCondition)
        result |= LayoutConstraint.HorizontallyFixed;
    
    return result; // Pure function
}

Benefits

  1. Cleaner Architecture: Eliminates side effects making constraint computation logic easier to reason about
  2. Better Encapsulation: Internal setter maintains access for specific scenarios while preventing unwanted modifications
  3. Improved Testability: Pure functions are easier to unit test
  4. Consistent Pattern: All constraint computation now follows the same return-value pattern

Compatibility

This change maintains full behavioral compatibility while improving the internal architecture. The constraint computation logic remains identical, only the mechanism for applying constraints has changed.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Change ComputeConstraintForView to return constraint and set ComputedConstraint via return value (.NET 10.0) Change ComputeConstraintForView to return LayoutConstraint instead of using side effects Aug 8, 2025
Copilot AI requested a review from PureWeen August 8, 2025 21:32
Copilot finished work on behalf of PureWeen August 8, 2025 21:32
@PureWeen PureWeen added this to the .NET 10.0-rc1 milestone Aug 12, 2025
@PureWeen PureWeen added the p/0 Current heighest priority issues that we are targeting for a release. label Aug 12, 2025
}

protected virtual void ComputeConstraintForView(View view) => view.ComputedConstraint = LayoutConstraint.None;
internal virtual LayoutConstraint ComputeConstraintForView(View view) => LayoutConstraint.None;
Copy link
Member

Choose a reason for hiding this comment

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

@copilot keep this as protected

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed ComputeConstraintForView from internal back to protected as requested in commit b9138ec. This maintains the existing inheritance pattern for custom layouts while preserving the return value refactoring.

Copy link
Contributor Author

Copilot AI commented Aug 12, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • services.gradle.org

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI requested a review from PureWeen August 12, 2025 06:34
Copilot finished work on behalf of PureWeen August 12, 2025 06:34
@github-project-automation github-project-automation bot moved this from Todo to Changes Requested in MAUI SDK Ongoing Aug 13, 2025
Copilot AI requested a review from PureWeen August 13, 2025 06:50
Copilot finished work on behalf of PureWeen August 13, 2025 06:50
@PureWeen
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen marked this pull request as ready for review August 13, 2025 07:54
Copilot AI review requested due to automatic review settings August 13, 2025 07:54
@PureWeen PureWeen requested a review from a team as a code owner August 13, 2025 07:54
@PureWeen PureWeen requested a review from jsuarezruiz August 13, 2025 07:54
Copy link
Contributor

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 PR refactors the ComputeConstraintForView method pattern across the MAUI layout system to eliminate side effects and improve code architecture. Previously, this method was a void function that directly modified the child view's ComputedConstraint property, creating side effects that made the code harder to reason about and test.

Key changes:

  • Changed ComputeConstraintForView from void to return LayoutConstraint value
  • Updated all layout classes to return constraint values instead of setting them directly
  • Modified calling code to assign the returned constraint value to the child's ComputedConstraint property

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Controls/src/Core/VisualElement/VisualElement.cs Base implementation change from void to return LayoutConstraint, updated callers to assign returned value
src/Controls/src/Core/TemplatedView/TemplatedView.cs Updated override to return constraint instead of setting directly
src/Controls/src/Core/TemplatedPage.cs Updated override to return constraint instead of setting directly
src/Controls/src/Core/ScrollView/ScrollView.cs Updated override to return constraint, added missing return for default case
src/Controls/src/Core/PublicAPI/*.txt Updated public API signatures to reflect method signature change
src/Controls/src/Core/ListView/ListView.cs Removed direct constraint assignment that's now handled by parent
src/Controls/src/Core/LegacyLayouts/StackLayout.cs Updated overrides and internal calls to use return values
src/Controls/src/Core/LegacyLayouts/Grid.cs Updated override to return constraint, fixed property change handler
src/Controls/src/Core/LegacyLayouts/AbsoluteLayout.cs Updated override to return constraint, simplified early return logic
src/Controls/src/Core/Layout/VerticalStackLayout.cs Updated override to return constraint instead of setting directly
src/Controls/src/Core/Layout/StackLayout.cs Updated override to return constraint instead of setting directly
src/Controls/src/Core/Layout/HorizontalStackLayout.cs Updated override to return constraint instead of setting directly
src/Controls/src/Core/Layout/Grid.cs Updated override to return constraint instead of setting directly
src/Controls/src/Core/ContentPresenter.cs Updated override to return constraint instead of setting directly

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@PureWeen PureWeen force-pushed the copilot/fix-32f9501c-499a-469a-81c7-249c01b63ff0 branch from 8ce8ce6 to 5f8b8bc Compare August 14, 2025 15:19
@PureWeen
Copy link
Member

/azp run

@PureWeen
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copilot AI requested a review from PureWeen August 16, 2025 16:16
Copilot finished work on behalf of PureWeen August 16, 2025 16:16
@PureWeen
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@github-project-automation github-project-automation bot moved this from Approved to Changes Requested in MAUI SDK Ongoing Aug 17, 2025
Copilot AI requested a review from PureWeen August 17, 2025 15:35
Copilot finished work on behalf of PureWeen August 17, 2025 15:35
@PureWeen PureWeen moved this from Changes Requested to Ready To Review in MAUI SDK Ongoing Aug 17, 2025
@PureWeen
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

Entry failure is unrelated

@PureWeen PureWeen merged commit 51b3a2d into net10.0 Aug 18, 2025
129 checks passed
@PureWeen PureWeen deleted the copilot/fix-32f9501c-499a-469a-81c7-249c01b63ff0 branch August 18, 2025 22:50
@github-project-automation github-project-automation bot moved this from Ready To Review to Done in MAUI SDK Ongoing Aug 18, 2025
@PureWeen PureWeen added the area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter label Aug 20, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Sep 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter p/0 Current heighest priority issues that we are targeting for a release.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants