Skip to content

Button: Revert sticky focus style#8767

Merged
henon merged 1 commit intoMudBlazor:devfrom
danielchalmers:button-focus
May 7, 2024
Merged

Button: Revert sticky focus style#8767
henon merged 1 commit intoMudBlazor:devfrom
danielchalmers:button-focus

Conversation

@danielchalmers
Copy link
Member

@danielchalmers danielchalmers commented Apr 20, 2024

Description

Currently if you click a button, it will apply the :focus style and keep it there until you click somewhere else. This was done in #8575 to make the dialog show focus on the right button but should be reverted until we figure out a better way of applying only the focus-visible pseudo class or at least not leaving the button pressed style indefinitely.

video2.mp4

Reverts #8575 #8709

How Has This Been Tested?

visually

Type 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)
  • Documentation (fix or improvement to the website or code docs)

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 the bug Unexpected behavior or functionality not working as intended label Apr 20, 2024
@danielchalmers
Copy link
Member Author

danielchalmers commented Apr 20, 2024

@Garderoben @LiZzeira

The button :focus styles were added in #8575 to show a visual style when the dialog calls Focus, but the :focus effect is visible after you click the button until you click somewhere else. We need to apply :focus-visible instead so it behaves like if you tabbed onto it and not use :focus.

I'm not aware of a JavaScript way of doing that and Blazor only has the plain Focus method.

See the current (faulty) behavior in action on dev.mudblazor.com after you click a button.


Goal: Apply the :focus-visible class when the dialog applies focus instead of :focus so we get the same style as if you tabbed onto the element. This means the same approach will work for all elements since we already use the :focus-visible styles.

@codecov
Copy link

codecov bot commented Apr 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.08%. Comparing base (28bc599) to head (ab9c38c).
Report is 90 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #8767      +/-   ##
==========================================
+ Coverage   89.82%   90.08%   +0.25%     
==========================================
  Files         412      419       +7     
  Lines       11878    12178     +300     
  Branches     2364     2397      +33     
==========================================
+ Hits        10670    10970     +300     
+ Misses        681      668      -13     
- Partials      527      540      +13     

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

@danielchalmers danielchalmers changed the title Dialog: Apply focus-visible instead of focus MudFocusTrap: Apply focus-visible instead of focus Apr 23, 2024
@danielchalmers danielchalmers changed the title MudFocusTrap: Apply focus-visible instead of focus Button: Revert sticky focus style May 5, 2024
@danielchalmers danielchalmers marked this pull request as ready for review May 5, 2024 23:56
@danielchalmers
Copy link
Member Author

@henon I couldn't find a better solution so I believe this total reversion should be merged until we discover something else. Otherwise the button will have a sticky style (unlike any other components).

@danielchalmers danielchalmers requested a review from henon May 5, 2024 23:58
@henon
Copy link
Contributor

henon commented May 6, 2024

@LiZzeira FYI, this will revert your PR.

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.

2 participants