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.

Sign up for GitHub

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

Jump to bottom

The fix for child FlexLayout content appearing no longer works starting from version 8.0.61 #26665

Merged
merged 2 commits into from
Dec 19, 2024
Merged

The fix for child FlexLayout content appearing no longer works starting from version 8.0.61 #26665

merged 2 commits into from
Dec 19, 2024

Conversation

Tamilarasan-Paranthaman
Copy link
Contributor

@Tamilarasan-Paranthaman Tamilarasan-Paranthaman commented Dec 17, 2024 •
edited
Loading

Root Cause of the issue

  • The issue occurs because the SelfSizing delegate for item is assigned during initialization via the AddFlexItem method, and it captures the value of InMeasureMode at that time. When InMeasureMode is later updated in CrossPlatformMeasure, the captured value inside the delegate remains unchanged (in this case, false) for the child FlexLayout content. This causes the else part of the logic to use the desired size, resulting in the content not being visible in the UI.

Description of Change

  • I fixed it by adding the boolean property as an argument to the delegate, which dynamically updates the changes to the boolean, and it works fine.

Regressed PR

Issues Fixed

Fixes #23491

Tested the behaviour in the following platforms

  • Windows
  • Android
  • iOS
  • Mac

Screenshot

Windows

Before Issue Fix After Issue Fix

Android

Before Issue Fix After Issue Fix

iOS

Before Issue Fix After Issue Fix

Mac

Before Issue Fix After Issue Fix

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Dec 17, 2024
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

Hey there @Tamilarasan-Paranthaman! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@Tamilarasan-Paranthaman Tamilarasan-Paranthaman added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Dec 17, 2024
@Tamilarasan-Paranthaman Tamilarasan-Paranthaman marked this pull request as ready for review December 17, 2024 11:55
@jfversluis
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@samhouts samhouts requested a review from Copilot December 17, 2024 18:18
Copilot
Copilot AI reviewed Dec 17, 2024
View reviewed changes
Copy link

@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.

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

Comments suppressed due to low confidence (1)

src/Controls/src/Core/Layout/FlexLayout.cs:588

  • Ensure that the new behavior introduced by the inMeasureMode parameter is covered by tests.
_root.Layout(InMeasureMode);

src/Core/src/Layouts/Flex.cs
}

public delegate void SelfSizingDelegate(Item item, ref float width, ref float height);
public delegate void SelfSizingDelegate(Item item, ref float width, ref float height, bool inMeasureMode);
Copy link
Preview

Copilot AI Dec 17, 2024

Choose a reason for hiding this comment

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

The change in the delegate signature is potentially a breaking change. Please ensure that this change is documented and communicated appropriately.

This comment was generated based on a coding guideline created by a repository admin.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@StephaneDelcroix
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

StephaneDelcroix
StephaneDelcroix reviewed Dec 18, 2024
View reviewed changes
Copy link
Contributor

@StephaneDelcroix StephaneDelcroix left a comment

Choose a reason for hiding this comment

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

I don't see the relation between the fix and the title (FlexLayout vs BindableLayout). could you please fix the PR subject ?

@Tamilarasan-Paranthaman Tamilarasan-Paranthaman changed the title Fix for BindableLayout.ItemsSource no longer works from 8.0.61 The fix for child FlexLayout content appearing no longer works starting from version 8.0.61 Dec 18, 2024
@Tamilarasan-Paranthaman
Copy link
Contributor Author

I don't see the relation between the fix and the title (FlexLayout vs BindableLayout). could you please fix the PR subject ?

@StephaneDelcroix, I have updated the PR title to reflect the fixed context. Could you please review it now?

@jfversluis jfversluis merged commit 6ea9765 into dotnet:main Dec 19, 2024
104 checks passed
@jfversluis jfversluis added this to the .NET 9 SR3 milestone Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot Copilot left review comments

@StephaneDelcroix StephaneDelcroix StephaneDelcroix approved these changes

@jfversluis jfversluis jfversluis approved these changes

@jsuarezruiz jsuarezruiz Awaiting requested review from jsuarezruiz

Assignees
No one assigned
Labels
community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration
Projects
None yet
Milestone
.NET 9 SR3
Development

Successfully merging this pull request may close these issues.

BindableLayout.ItemsSource no longer works in 8.0.61
3 participants
@Tamilarasan-Paranthaman @jfversluis @StephaneDelcroix