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

Make sure ImageButton always has a background #22717

Merged
merged 1 commit into from
Jun 5, 2024
Merged

Make sure ImageButton always has a background #22717

merged 1 commit into from
Jun 5, 2024

Conversation

mattleibow
Copy link
Member

@mattleibow mattleibow commented May 29, 2024

Description of Change

The OS does not specify a background so we need to make sure there is a transparent background to get the ripple and border.

We can't really test ripple and we can't really test a null background vs a transparent background as they are supposed to look exactly the same.

The original issue was observing a lack of ripple on white, but this is because the ripple is white. In traditional buttons, there is a RippleColor property on the platform view. However, on image buttons, the control is just an image view so does not have any ripple effects - we are adding it. It will have to be a separate platform effect to add a ripple color property.

See #22805

Issues Fixed

Fixes #22603

Before

Notice the lack of a ripple on the top right button

before.webm

After

Now the top right button has a ripple

after.webm
DEMO CODE
  <Grid ColumnDefinitions="*,*">

      <VerticalStackLayout Padding="30"
                           Spacing="10"
                           Background="White"
                           Grid.Column="0">
          <Label Text="NULL Background"
                 TextColor="Gray"/>
          <ImageButton Aspect="Center"
                       CornerRadius="8"
                       HeightRequest="80"
                       WidthRequest="80"
                       Source="{FontImage FontFamily='Ionicons', Glyph='&#xF32B;', Color='DarkOrange', Size='60'}"/>
          <Label Text="TRANSPARENT Background"
                 TextColor="Gray"/>
          <ImageButton Background="Transparent"
                       Aspect="Center"
                       CornerRadius="8"
                       WidthRequest="80"
                       HeightRequest="80"
                       Source="{FontImage FontFamily='Ionicons', Glyph='&#xF32B;', Color='DarkOrange', Size='60'}"/>
          <Label Text="WHITE Background"
                 TextColor="Gray"/>
          <ImageButton Background="White"
                       Aspect="Center"
                       CornerRadius="8"
                       WidthRequest="80"
                       HeightRequest="80"
                       Source="{FontImage FontFamily='Ionicons', Glyph='&#xF32B;', Color='DarkOrange', Size='60'}"/>
          <Label Text="COLOR Background"
                 TextColor="Gray"/>
          <ImageButton Background="DarkSlateGray"
                       Aspect="Center"
                       CornerRadius="8"
                       WidthRequest="80"
                       HeightRequest="80"
                       Source="{FontImage FontFamily='Ionicons', Glyph='&#xF32B;', Color='DarkOrange', Size='60'}"/>
      </VerticalStackLayout>

      <VerticalStackLayout Padding="30"
                           Spacing="10"
                           Background="Black"
                           Grid.Column="1">
          <Label Text="NULL Background"
                 TextColor="Gray"/>
          <ImageButton Aspect="Center"
                       CornerRadius="8"
                       HeightRequest="80"
                       WidthRequest="80"
                       Source="{FontImage FontFamily='Ionicons', Glyph='&#xF32B;', Color='DarkOrange', Size='60'}"/>
          <Label Text="TRANSPARENT Background"
                 TextColor="Gray"/>
          <ImageButton Background="Transparent"
                       Aspect="Center"
                       CornerRadius="8"
                       WidthRequest="80"
                       HeightRequest="80"
                       Source="{FontImage FontFamily='Ionicons', Glyph='&#xF32B;', Color='DarkOrange', Size='60'}"/>
          <Label Text="WHITE Background"
                 TextColor="Gray"/>
          <ImageButton Background="White"
                       Aspect="Center"
                       CornerRadius="8"
                       WidthRequest="80"
                       HeightRequest="80"
                       Source="{FontImage FontFamily='Ionicons', Glyph='&#xF32B;', Color='DarkOrange', Size='60'}"/>
          <Label Text="COLOR Background"
                 TextColor="Gray"/>
          <ImageButton Background="DarkSlateGray"
                       Aspect="Center"
                       CornerRadius="8"
                       WidthRequest="80"
                       HeightRequest="80"
                       Source="{FontImage FontFamily='Ionicons', Glyph='&#xF32B;', Color='DarkOrange', Size='60'}"/>
      </VerticalStackLayout>

  </Grid>

The OS does not specify a background so we need to make sure there is
a transparent background to get the ripple and border.
@mattleibow mattleibow requested a review from a team as a code owner May 29, 2024 14:40
@mattleibow mattleibow requested review from Eilon and jfversluis May 29, 2024 14:40
@jsuarezruiz jsuarezruiz added the area-controls-button Button, ImageButton label May 31, 2024
@@ -76,7 +77,9 @@ internal static void UpdateButtonStroke(this ShapeableImageView platformView, IB

internal static void UpdateButtonBackground(this ShapeableImageView platformView, IImageButton button)
{
platformView.UpdateMauiRippleDrawableBackground(button,
platformView.UpdateMauiRippleDrawableBackground(
button.Background ?? new SolidPaint(Colors.Transparent), // transparent to force some background
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use the same default color applied to Button?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure this ever was the case, so it may be a breaking change?

Copy link
Member

Choose a reason for hiding this comment

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

Was it always Transparent ? Should we grab the color from the theme here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me test OG net8 and see. I am sure I have always seen transparent because this is not a button, it is an image view. I will also test net7.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tested OG 8.0.3 and there is no background. Same with 7.0.101+4.

@Syed-RI
Copy link

Syed-RI commented May 31, 2024

Do you have any before and after video of the effect?

@mattleibow
Copy link
Member Author

I added a video for the ripple, a bit hard to see as modern Androids have a very subtle ripple. But, I also opened an issue for a platform specific property: #22805

@PureWeen PureWeen requested review from jsuarezruiz and rmarinho June 4, 2024 14:05
Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

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

issue-22603

@MauiUIui
Copy link

This might have just created this bug: #23119

@github-actions github-actions bot locked and limited conversation to collaborators Jul 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Button/ImageButton RippleEffect not working in some cases
6 participants