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

Fix [Android] Image AspectFill is not honored #25072

Merged
merged 32 commits into from
Jan 8, 2025

Conversation

devanathan-vaithiyanathan
Copy link
Contributor

@devanathan-vaithiyanathan devanathan-vaithiyanathan commented Oct 3, 2024

Root Cause

In Image Android Handler, set true for SetAdjustViewBounds(), when you enable view bounds adjustment, the ImageView will automatically resize to maintain the image's aspect ratio.

Description of Change

This update fixes image aspect handling for Android in ImageView, If the aspect is AspectFill, set the false for SetAdjustViewBounds(), so the view bounds are not adjusted, allowing the image to fill the view.

Tested the behaviour in the following platforms

  • Android
  • Windows
  • iOS
  • Mac

Issues Fixed

Fixes #21368

Output Screenshot

Before After
Android
Android

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Oct 3, 2024
@jsuarezruiz jsuarezruiz added platform/android 🤖 area-image Image loading, sources, caching labels Oct 4, 2024
@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@devanathan-vaithiyanathan devanathan-vaithiyanathan marked this pull request as ready for review October 4, 2024 13:24
@devanathan-vaithiyanathan devanathan-vaithiyanathan requested a review from a team as a code owner October 4, 2024 13:24
@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Oct 14, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@@ -17,6 +17,11 @@ public static void Clear(this ImageView imageView)

public static void UpdateAspect(this ImageView imageView, IImage image)
{
if (image.Aspect is Aspect.AspectFill)
Copy link
Member

Choose a reason for hiding this comment

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

If I delete this code the test still passes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A new test has been added, and now the test only passes when the fix is applied. This ensures that the fix is correctly addressing the issue, and without it, the test fails as expected.

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen added this to the .NET 9 Servicing milestone Oct 31, 2024
if (image.Aspect is Aspect.AspectFill)
imageView.SetAdjustViewBounds(false);
else
imageView.SetAdjustViewBounds(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

The changes are impacting other related Image UITests:
Issue18242Test
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’ve modified the fix and rerun the mentioned test. The test passed successfully without any issues.
Also I have committed the same. Could you please check and let me know.

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@Redth Redth requested a review from Copilot December 4, 2024 21:48

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 7 changed files in this pull request and generated no suggestions.

Files not reviewed (1)
  • src/Controls/tests/TestCases.HostApp/Issues/Issue21368.xaml: Language not supported
Comments skipped due to low confidence (2)

src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue21368.cs:30

  • Remove the extra space before the #endif directive.
#endif

src/Core/src/Platform/Android/ImageViewExtensions.cs:19

  • The new behavior in the UpdateAspect method should have explicit test coverage to ensure the aspect handling logic is verified.
public static void UpdateAspect(this ImageView imageView, IImage image)
@sheiksyedm sheiksyedm added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Dec 6, 2024
@PureWeen
Copy link
Member

PureWeen commented Jan 6, 2025

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen modified the milestones: .NET 9 Servicing, .NET 9 SR3 Jan 8, 2025
@PureWeen
Copy link
Member

PureWeen commented Jan 8, 2025

  • failing test unrelated

@PureWeen PureWeen merged commit 35f1ee6 into dotnet:main Jan 8, 2025
111 of 113 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-image Image loading, sources, caching community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration platform/android 🤖
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Android] image AspectFill is not honored
4 participants