Skip to content

MudButton: fix focus style button when open dialog#8575

Merged
Garderoben merged 1 commit intoMudBlazor:devfrom
LiZzeira:patch-1
Apr 8, 2024
Merged

MudButton: fix focus style button when open dialog#8575
Garderoben merged 1 commit intoMudBlazor:devfrom
LiZzeira:patch-1

Conversation

@LiZzeira
Copy link
Contributor

@LiZzeira LiZzeira commented Apr 5, 2024

Description

The style file for the MudButton components has been updated to include the :focus and :focus-visible pseudo-classes, applying them to the respective buttons.

How Has This Been Tested?

I made a style modification to ensure that the focus on the button is correctly applied when the dialog opens.

This modification addresses several issues, such as:
#4283
#8451

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)

Before:

Dialog.-.MudBlazor.-.Google.Chrome.2024-04-04.22-42-50.mp4

After:
https://github.com/MudBlazor/MudBlazor/assets/79289665/e0609b70-9899-48c0-90de-61f2d585b468
https://github.com/MudBlazor/MudBlazor/assets/79289665/6877c9d1-dd11-496f-bdbf-9cd41498c8b5
https://github.com/MudBlazor/MudBlazor/assets/79289665/07386ced-3695-427b-bacd-10e03e15cabd

Checklist:

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

@github-actions github-actions bot added bug Unexpected behavior or functionality not working as intended PR: needs review labels Apr 5, 2024
@LiZzeira LiZzeira mentioned this pull request Apr 5, 2024
6 tasks
@codecov
Copy link

codecov bot commented Apr 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.77%. Comparing base (e44b9f3) to head (7147839).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #8575      +/-   ##
==========================================
+ Coverage   89.75%   89.77%   +0.02%     
==========================================
  Files         411      411              
  Lines       11817    11817              
  Branches     2362     2362              
==========================================
+ Hits        10606    10609       +3     
+ Misses        683      681       -2     
+ Partials      528      527       -1     

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

@danielchalmers
Copy link
Member

Just fyi your sound is on in the clips

@danielchalmers danielchalmers self-requested a review April 5, 2024 02:01
@LiZzeira
Copy link
Contributor Author

LiZzeira commented Apr 5, 2024

lol
Sorry, I hadn't seen it but it's okay

@danielchalmers danielchalmers requested a review from henon April 5, 2024 02:08
@danielchalmers
Copy link
Member

Nice work, again. Thanks for looking into this.

I do wonder why focus-visible is used alone in so many controls though? Must be a reason. @Garderoben may know.
https://github.com/search?q=repo%3AMudBlazor%2FMudBlazor%20focus-visible&type=code

@henon
Copy link
Contributor

henon commented Apr 5, 2024

I want @Garderoben to double check this to be sure.

@ScarletKuro ScarletKuro requested a review from Garderoben April 5, 2024 10:56
@LiZzeira
Copy link
Contributor Author

LiZzeira commented Apr 6, 2024

When using the :focus-visible pseudo-class, the applied style will only be visible if the focused element is via keyboard, such as when pressing the "Tab" key to toggle between buttons.

@Garderoben Garderoben merged commit 42b3803 into MudBlazor:dev Apr 8, 2024
@danielchalmers
Copy link
Member

@LiZzeira Do you think we should be looking at any of the other components that use focus-visible, or is the button enough? https://github.com/search?q=repo%3AMudBlazor%2FMudBlazor%20focus-visible&type=code

@LiZzeira
Copy link
Contributor Author

LiZzeira commented Apr 9, 2024

@danielchalmers I'll conduct some tests on these additional components, and if needed, I'll update and push a new PR

danielchalmers added a commit to danielchalmers/MudBlazor that referenced this pull request Apr 15, 2024
Fixes a regression of MudBlazor#8256 caused by MudBlazor#8575

Not perfect but probably the best with the tools we have.
danielchalmers added a commit to danielchalmers/MudBlazor that referenced this pull request Apr 20, 2024
biegehydra pushed a commit to biegehydra/MudBlazor that referenced this pull request Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Unexpected behavior or functionality not working as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants