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

[Android] Fix Ripple effect with custom background (alternative to #17821) #20412

Merged
merged 19 commits into from Apr 2, 2024

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Feb 7, 2024

Description of Change

Fix Ripple effect with custom background (alternative to #17821).

Ripple effects (without background, using backgound color, using a brush, etc)
fix-17642-ripple

Modifying also borders, corners etc
fix-17642

To test, use the added sample to the UITest project.

Also added a device test.
image

Issues Fixed

Fixes #17642

@jsuarezruiz jsuarezruiz added t/bug Something isn't working platform/android 🤖 area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing area-controls-button Button, ImageButton labels Feb 7, 2024
@jsuarezruiz jsuarezruiz requested a review from a team as a code owner February 7, 2024 07:58
@ghost ghost added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Feb 7, 2024
@jsuarezruiz jsuarezruiz marked this pull request as draft February 7, 2024 10:04
@jsuarezruiz jsuarezruiz marked this pull request as ready for review February 7, 2024 12:47
base.DisconnectHandler(platformView);
}

// This is a Android-specific mapping
public static void MapBackground(IButtonHandler handler, IButton button)
{
handler.PlatformView?.UpdateBackground(button);
if (handler is ButtonHandler buttonHandler)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to case the handler. The PlatformView on IButtonHandler is MaterialButton

using PlatformView = Google.Android.Material.Button.MaterialButton;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, but require the cast to ButtonHandler to access to the default drawable and pass it updating the Background.

@jsuarezruiz
Copy link
Contributor Author

@Redth @PureWeen Can we prioritize this issue and include it in the next SR (with this fix or a different approach)?

@jsuarezruiz
Copy link
Contributor Author

The failing Windows device test is unrelated
image

@mattleibow mattleibow self-assigned this Mar 15, 2024
@PureWeen
Copy link
Member

@Redth @PureWeen Can we prioritize this issue and include it in the next SR (with this fix or a different approach)?

Yea, @mattleibow is going to get this one buttoned up.

@PureWeen
Copy link
Member

/rebase

src/Core/src/Graphics/PaintExtensions.Android.cs Outdated Show resolved Hide resolved
}
}
// Default obtained by running on Android 34 and inspecting the default drawable
const int DefaultCornerRadius = 4;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are these values correct for all supported API levels?

Copy link
Member

Choose a reason for hiding this comment

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

I copied from 34, so maybe... I was hoping consistency is better but we can check later maybe so we can get this PR in.

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 material, so most likely the values are the same across all OS versions - but I have not checked.

gradientDrawable?.SetColor(ColorStateList.ValueOf(solidPaint.Color.ToPlatform()));
}
// Copy the tints from a temporary button.
using var btn = new MaterialButton(context);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Passing the default drawable has been changed to copy the tints from a temporary button. It is not an operation that happens frequently, set a background and then removing it, but is this perhaps more expensive?

Copy link
Member

Choose a reason for hiding this comment

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

It is a bit more expensive, but we don't need to add fields to handlers and pass the default drawable around. That requires new APIs and a bunch of old ones not working. It is more expensive, but this is smei-temporary because all I am using is the tint:

var defaultTintList = btn.BackgroundTintList;
var defaultColor = defaultTintList?.GetColorForState([R.Attribute.StateEnabled], AColor.Black) ?? AColor.Black;

A better option is to not do either and just figure out the default tint color. There is probably some theme resource or some API to just get the raw color. However the color goes from android resources to the main color of the button - some primary color thing. If we can do that in our code, it will just be reading a resource color from the android resources.

I'll create an issue to fix this.

Copy link
Member

Choose a reason for hiding this comment

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

@PureWeen
Copy link
Member

PureWeen commented Apr 2, 2024

My change was just fixing the Tizen API shipping txt file nothing functional.

@PureWeen PureWeen merged commit 49676d2 into main Apr 2, 2024
47 checks passed
@PureWeen PureWeen deleted the fix-17642-III branch April 2, 2024 15:22
@OvrBtn
Copy link

OvrBtn commented Apr 2, 2024

Hello, approximately when can we expect new MAUI version with this fix?

@AlleSchonWeg
Copy link

Hi @PureWeen and @jsuarezruiz ,
this issue is not completely fixed. You closed this #18857 (comment) as duplicate and link this issue: #17642 which closes this PR. But ImageButton has still no ripple effect. Normal button works.

@PureWeen
Copy link
Member

PureWeen commented Apr 4, 2024

Hello, approximately when can we expect new MAUI version with this fix?

Hopefully next week. In the mean time can you test the nightly and let us know how it works for you?

https://github.com/dotnet/maui/wiki/Nightly-Builds

@OvrBtn
Copy link

OvrBtn commented Apr 4, 2024

Hello, approximately when can we expect new MAUI version with this fix?

Hopefully next week. In the mean time can you test the nightly and let us know how it works for you?

https://github.com/dotnet/maui/wiki/Nightly-Builds

I've created a copy of my current project and I've upgraded it from .NET 7 to newest .NET 8 nightly build: 8.0.20-nightly.10431

I don't really have the ability to test iOS, but on Android buttons work just like in .NET 7 which is great :)

@rudyspano
Copy link

rudyspano commented Apr 18, 2024

Hi @PureWeen .
How to simply change the White Color to another Color ? (Light mode...)

@rudyspano
Copy link

rudyspano commented Apr 19, 2024

My solution:

            ButtonHandler.Mapper.AppendToMapping("RippleEffectCustomization", (handler, view) =>
            {
                //cf: https://github.com/dotnet/maui/blob/main/src/Core/src/Platform/Android/ButtonExtensions.cs
                const float DefaultRippleAlpha = 0.24f;

                var button = handler.PlatformView;

                if (button.Background is not RippleDrawable rippleDrawable)
                {
                    return;
                }

              ButtonHandler.Mapper.AppendToMapping("RippleEffectCustomization", (handler, view) =>
              {
                  //cf: https://github.com/dotnet/maui/blob/main/src/Core/src/Platform/Android/ButtonExtensions.cs
                  const float DefaultRippleAlpha = 0.24f;
  
                  var button = handler.PlatformView;
  
  
                  if (button.Background is not RippleDrawable rippleDrawable)
                  {
                      return;
                  }
  
                  rippleDrawable.SetColor(ColorStateList.ValueOf(Colors.Yellow.WithAlpha(DefaultRippleAlpha).ToPlatform()));
              });

@PureWeen , is there a simpler way ?

It overrides this part of code. I cannot set platformView.RippleColor (always null after setting)

image

@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-button Button, ImageButton area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing platform/android 🤖 t/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[regression/8.0.0-preview.6.8686] Button Click Ripple Effect Not Working In Android
7 participants