Skip to content

Conversation

@jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Jul 14, 2023

Description of Change

Fix Button IconSize on Android.

Before

After
Captura de pantalla 2023-07-12 a las 14 14 57

To validate the changes, launch the .NET MAUI Gallery and navigate to the Button samples. Check the added samples using the huge dotnet bot png.

Captura de pantalla 2023-07-17 a las 9 24 32 Captura de pantalla 2023-07-17 a las 11 34 18

Issues Fixed

Fixes #12054

@jsuarezruiz jsuarezruiz added t/bug Something isn't working platform/android area-controls-button Button, ImageButton area-controls-image Image control labels Jul 14, 2023
@ghost ghost added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Jul 14, 2023
@jfversluis
Copy link
Member

From the images it looks like the aspect is not preserved (it's stretched or squashed) is that something we should do?

if (Handler is ButtonHandler buttonHandler && buttonHandler.PlatformView is MaterialButton materialButton)
{
_materialButton = materialButton;
_materialButton.LayoutChange += OnButtonLayoutChange;
Copy link
Member

Choose a reason for hiding this comment

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

This is going to cause memory leaks.

Can we trigger this logic from the handler without events?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reviewed and added a way to always unsubscribe with test to validate that the Button does not leak.
Captura de pantalla 2023-07-17 a las 11 34 18

@jsuarezruiz
Copy link
Contributor Author

Added device test
Captura de pantalla 2023-07-17 a las 9 24 32

@jsuarezruiz jsuarezruiz requested a review from PureWeen July 17, 2023 09:36
@jsuarezruiz jsuarezruiz marked this pull request as ready for review August 3, 2023 14:14
jfversluis
jfversluis previously approved these changes Aug 15, 2023
@samhouts samhouts added this to the Under Consideration milestone Aug 15, 2023
@jsuarezruiz jsuarezruiz requested a review from a team as a code owner August 16, 2023 10:51
@samhouts samhouts added the stale Indicates a stale issue/pr and will be closed soon label Sep 11, 2023
@jsuarezruiz jsuarezruiz removed the stale Indicates a stale issue/pr and will be closed soon label Oct 6, 2023

private protected override void OnHandlerChangedCore()
{
base.OnHandlerChangedCore();
Copy link
Member

Choose a reason for hiding this comment

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

This isn't going to always fire. Can we make this work without needing to subscribe to LayoutChange from the controls code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are also unsubscribing when the parent Window is null. I have added a test to validate leaks. What would be a possible scenario for testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed and start using ArrangeOverride

@jsuarezruiz jsuarezruiz requested a review from PureWeen November 2, 2023 15:07
@samhouts samhouts added the stale Indicates a stale issue/pr and will be closed soon label Nov 21, 2023
@jsuarezruiz jsuarezruiz removed the stale Indicates a stale issue/pr and will be closed soon label Jan 8, 2024
@samhouts samhouts added the stale Indicates a stale issue/pr and will be closed soon label Feb 6, 2024
@PureWeen PureWeen closed this Mar 18, 2024
@PureWeen
Copy link
Member

Fixed by
#19834

@github-actions github-actions bot locked and limited conversation to collaborators Apr 18, 2024
@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
@samhouts samhouts removed this from the Under Consideration milestone Jul 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-controls-button Button, ImageButton area-controls-image Image control platform/android stale Indicates a stale issue/pr and will be closed soon t/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Android] A big image overflow the Button

6 participants