KEMBAR78
Android/iOS Button ImageSource by PureWeen · Pull Request #2079 · dotnet/maui · GitHub
Skip to content

Conversation

PureWeen
Copy link
Member

@PureWeen PureWeen commented Aug 13, 2021

Description of Change

  • Adds full implementation of ContentLayout and ImageSource to Android and iOS
  • I made all the IButtonContentLayout bits internal for now. We can discuss making them public in a later PR
  • This converts Android to using MaterialButton which has built in behavior for laying out the icon next to the text. Unfortunately they implement it for all three sides except for Bottom which made the implementation a bit odd
  • For Android I converted our FileImageSourceHandler to use GetDrawable for now so that it returns the image at the same size as iOS. We'll reconcile all the Glide behavior in a separate PR
  • Currently on XF iOS sets the image to Aspect Fit whereas android just loads the image as is. I changed this to Center on iOS so it matches Android. Currently on XF iOS if you load an image that's too large for the button it looks completely wrong. It also looks wrong in MAUI but it looks less wrong. Fixing cases where the image is too large will be part of a different PR

Known Issues

Android still has some measuring issues that need to be worked through. The buttons with images on the start/end appear to have issues when nested inside a NavigationPage. We're going to replace the NavigationPage with a better Maui.Core implementation so for now we will probably wait until that implementation is in to revisit the issues here. I did a quick test using the Android Navigation View handler branch and on that branch the layouts were all correct.

Breaking Changes

This implementation breaks iOS but makes it match with Android so now the ContentLayout behavior between iOS and Android should be identical. The ultimate fix here would be to add some additional layout position options (LeftText, RightText, TopText, BottomText) . The implementation in XF on iOS is basically LeftText, RightText, TopText, BottomText.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)
  • Targets a single property for a single control (or intertwined few properties)
  • Adds the property to the appropriate interface
  • Avoids any changes not essential to the handler property
  • Adds the mapping to the PropertyMapper in the handler
  • Adds the mapping method to the Android, iOS, and Standard aspects of the handler
  • Implements the actual property updates (usually in extension methods in the Platform section of Core)
  • Tags ported renderer methods with [PortHandler]
  • Adds an example of the property to the sample project (MainPage)
  • Adds the property to the stub class
  • Implements basic property tests in DeviceTests

Does this PR touch anything that might affect accessibility?

  • Does this PR introduce a new control? (If yes, add an example using SemanticProperties to the SemanticsPage)
  • APIs that modify focusability?
  • APIs that modify any text property on a control?
  • Does this PR modify view nesting or view arrangement in anyway?
  • Is there the smallest possibility that your PR will change accessibility?
  • I'm not sure, please help me

If any of the above checkboxes apply to your PR, then the PR will need to provide testing to demonstrate that accessibility still works.

@PureWeen PureWeen changed the title Button images Android Button ImageSource Aug 13, 2021

namespace Microsoft.Maui
{
internal partial class ImageSourcePartWrapper<T> : IImageSourcePart
Copy link
Member Author

Choose a reason for hiding this comment

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

This generalizes code for loading an image so that all code can benefit from cancel behavior and cases where the destination goes poof

@PureWeen PureWeen marked this pull request as ready for review August 13, 2021 02:58
@PureWeen PureWeen requested a review from mattleibow August 13, 2021 15:36
@PureWeen PureWeen changed the title Android Button ImageSource Android/iOS Button ImageSource Aug 14, 2021
@PureWeen
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@Redth Redth added this to the 6.0.100-rc.1 milestone Aug 23, 2021
Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

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

D:\agent\1\s\src\Controls\src\Core\HandlerImpl\Button\Button.Impl.cs(26,23): error CS1061: 'MauiButton' does not contain a definition for 'UpdateContentLayout' and no accessible extension method 'UpdateContentLayout' accepting a first argument of type 'MauiButton' could be found (are you missing a using directive or an assembly reference?) [D:\agent\1\s\src\Controls\src\Core\Controls.Core-net6.csproj]

@PureWeen PureWeen requested a review from rmarinho August 25, 2021 12:58
@mattleibow mattleibow dismissed rmarinho’s stale review August 25, 2021 22:47

Fixed I think?

@mattleibow mattleibow merged commit 8b16750 into main Aug 25, 2021
@mattleibow mattleibow deleted the button_images branch August 25, 2021 22:53
@github-actions github-actions bot locked and limited conversation to collaborators Dec 25, 2023
@samhouts samhouts added the fixed-in-6.0.100-rc.1.7 Look for this fix in 6.0.100-rc.1.7! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

fixed-in-6.0.100-rc.1.7 Look for this fix in 6.0.100-rc.1.7! t/breaking 💥

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants