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

MudRipple: Follow theme colors and improve visibility (#8072) #8460

Merged
merged 4 commits into from
Mar 26, 2024

Conversation

meenzen
Copy link
Contributor

@meenzen meenzen commented Mar 25, 2024

Description

Previously the ripple effect would only use a static color (#000) which doesn't work well with the dark theme. Additionally it was hard to see on filled buttons.

Changes:

  • Always use the text color as the ripple color
  • Double the effect opacity for filled buttons
  • Add a palette option for the ripple opacity
  • Fix a minor visual issue where MudNavLink ripple effect was too large

Note: Other Material UI libraries also use the text color for the ripple effect.

resolves #8072

How Has This Been Tested?

visually

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Filled Buttons

Before:

Screencast_20240325_130427.webm

After:

Screencast_20240325_130518.webm

Text Buttons

Before:

Screencast_20240325_130742.webm

After:

Screencast_20240325_130811.webm

MudNavLink

Before:

image

After:

image

Checklist:

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

This made the ripple effect a few pixels larger than the clicked link, which looked very odd.
@github-actions github-actions bot added bug Something does not work as intended/expected enhancement New feature or request PR: needs review labels Mar 25, 2024
Copy link

codecov bot commented Mar 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.01%. Comparing base (61e8a76) to head (c664b36).
Report is 7 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #8460      +/-   ##
==========================================
+ Coverage   88.84%   89.01%   +0.16%     
==========================================
  Files         416      411       -5     
  Lines       12358    12188     -170     
  Branches     2458     2433      -25     
==========================================
- Hits        10980    10849     -131     
+ Misses        845      803      -42     
- Partials      533      536       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielchalmers
Copy link
Contributor

danielchalmers commented Mar 25, 2024

Looks great! Naturally handles light/dark themes and makes the hover->active->ripple effect less jarring.
Can you confirm it looks good with touch on mobile too?

I think --mud-ripple-opacity: 0.2 should be scaled so one could change the base value and have it propagate through the library.

I personally think the hover and ripple should be more opaque like MUI but that's a separate issue.

Copy link
Collaborator

@henon henon left a comment

Choose a reason for hiding this comment

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

I love it.

@danielchalmers
Copy link
Contributor

Should probably be configurable through the Palette

public double HoverOpacity { get; set; } = 0.06;

@henon
Copy link
Collaborator

henon commented Mar 26, 2024

Should probably be configurable through the Palette

I don't expect many will need to customize it but who knows. Would this be easy to do as part of this PR?

@meenzen
Copy link
Contributor Author

meenzen commented Mar 26, 2024

I've added configuration options to the palette, both opacity values can be changed separately.

@henon henon merged commit 7be1b80 into MudBlazor:dev Mar 26, 2024
3 checks passed
@henon
Copy link
Collaborator

henon commented Mar 26, 2024

Thanks @meenzen

@meenzen meenzen deleted the feat/ripple-color-visibility branch March 26, 2024 12:35
@danielchalmers
Copy link
Contributor

Amazing, thanks for doing that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as intended/expected enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improved ripple effect contrast / colors
4 participants