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

FocusTrap: Fix outline focus #7835

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

LiZzeira
Copy link
Contributor

@LiZzeira LiZzeira commented Nov 30, 2023

Description

Locate the problem within the MudFocusTrap component. You can typically find the relevant code in the Razor component file.

In the Razor component file, navigate to the specific line (e.g., line 22) where the MudFocusTrap component is defined using the

tag.

Within this

element, you'll want to add an existing CSS class called "outline-none." This class is commonly available in MudBlazor and is used to remove unwanted outlines or focus styles.

Before:
https://github.com/MudBlazor/MudBlazor/assets/79289665/e197d41d-5847-4943-8e09-7d5f7a2f2f93

After:
https://github.com/MudBlazor/MudBlazor/assets/79289665/fa4ab9b6-325b-477a-b8c1-e8b385fce841

How Has This Been Tested?

To test the problem:

Open a web browser that uses the Chromium engine, such as Google Chrome or Microsoft Edge.

Load the web page or web application that contains the MudFocusTrap component you want to test.

Open the dialog or window that contains the MudFocusTrap component. Ensure that the dialog is visible on the screen.

Within the content of the dialog, click on an area where there are no other interactive components, such as buttons or text input fields.

Press any key on the keyboard. Notice that an unwanted focus or outline may become visible around the MudFocusTrap component.

Dialog - MudBlazor - Google Chrome 2023-11-30 10-10-48.zip

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)

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 Something does not work as intended/expected PR: needs review labels Nov 30, 2023
@mikes-gh mikes-gh changed the title fix: outiline focus in focusTrap component FocusTrap: Fix outline focus Feb 1, 2024
@ScarletKuro
Copy link
Member

Hi.

Thanks for PR.
Sorry that it took so long to notice the PR...
Could you post a before and after picture or a gif?

@LiZzeira
Copy link
Contributor Author

LiZzeira commented Apr 3, 2024

Hello.

Here are the videos

Before: https://github.com/MudBlazor/MudBlazor/assets/79289665/e197d41d-5847-4943-8e09-7d5f7a2f2f93

After: https://github.com/MudBlazor/MudBlazor/assets/79289665/fa4ab9b6-325b-477a-b8c1-e8b385fce841

Copy link
Contributor

@danielchalmers danielchalmers left a comment

Choose a reason for hiding this comment

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

Tested it out and is a nice improvement

@danielchalmers
Copy link
Contributor

I wonder if the focus trap is part of the cause of #4283

@LiZzeira
Copy link
Contributor Author

LiZzeira commented Apr 3, 2024

it seems to be the solution to this problem

@henon
Copy link
Collaborator

henon commented Apr 4, 2024

@LiZzeira are you saying this PR closes #4283 ? Are you sure about that?

@LiZzeira
Copy link
Contributor Author

LiZzeira commented Apr 4, 2024

@henon I'm not sure, but I'm testing this case

@danielchalmers
Copy link
Contributor

it seems to be the solution to this problem

Would be so great if it worked because it's been causing issues for years. Another example in #8451

@LiZzeira
Copy link
Contributor Author

LiZzeira commented Apr 4, 2024

I found the cause of this problem; it's not in the FocusTrap but in the style applied to the MudButton. I will try to solve this problem in the best way possible and push a new update.

@henon
Copy link
Collaborator

henon commented Apr 4, 2024

@LiZzeira that would be kind of you. Just open another PR please, I am merging this one.

@henon henon merged commit d7a5f8f into MudBlazor:dev Apr 4, 2024
4 checks passed
@LiZzeira
Copy link
Contributor Author

LiZzeira commented Apr 5, 2024

@henon @danielchalmers
I made the correction regarding the problem with the focus on the button

@LiZzeira
Copy link
Contributor Author

LiZzeira commented Apr 5, 2024

New PR open
#8575

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 Something does not work as intended/expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants