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

Dialog: allow to override focus trap behaviour #3539

Merged
merged 2 commits into from
Feb 28, 2022

Conversation

steveoandrey
Copy link
Contributor

Description

The MudDialog uses a FocusTrap, but it is not accesible to the programmer. The use case a colleague had was that he didn't want to have the first element focused. This PR exposes the FocusTrap-Setting in the dialog.

How Has This Been Tested?

Only tested visually, could not figure out how to unit test this.

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. -> help needed

@codecov
Copy link

codecov bot commented Dec 12, 2021

Codecov Report

Merging #3539 (5e3218c) into dev (52d811f) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #3539      +/-   ##
==========================================
+ Coverage   91.12%   91.13%   +0.01%     
==========================================
  Files         351      351              
  Lines       11623    11623              
==========================================
+ Hits        10591    10593       +2     
+ Misses       1032     1030       -2     
Impacted Files Coverage Δ
src/MudBlazor/Components/Dialog/MudDialog.razor 100.00% <ø> (ø)
src/MudBlazor/Components/Dialog/MudDialog.razor.cs 94.73% <ø> (ø)
...MudBlazor/Components/DataGrid/MudDataGrid.razor.cs 91.98% <0.00%> (+0.42%) ⬆️
...dBlazor/Components/FocusTrap/MudFocusTrap.razor.cs 56.52% <0.00%> (+2.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52d811f...5e3218c. Read the comment docs.

@steveoandrey steveoandrey marked this pull request as ready for review December 18, 2021 10:55
@mikes-gh mikes-gh changed the title MudDialog: allow to override focus trap behaviour Dialog: allow to override focus trap behaviour Dec 19, 2021
@JonBunator JonBunator added enhancement New feature or request needs review labels Dec 22, 2021
@mikes-gh mikes-gh added the API change API that needs approval label Jan 10, 2022
@henon
Copy link
Collaborator

henon commented Feb 25, 2022

This looks good. The only thing missing is a test case for the new API so that our test coverage isn't reduced by the PR.

@henon
Copy link
Collaborator

henon commented Feb 28, 2022

Ah coverage looks good. Merging ...

@henon henon merged commit 69e9474 into MudBlazor:dev Feb 28, 2022
@henon
Copy link
Collaborator

henon commented Feb 28, 2022

Thanks!

@henon henon removed the needs tests label Feb 28, 2022
@henon henon added this to the 6.0.8 milestone Feb 28, 2022
@steveoandrey
Copy link
Contributor Author

Sorry I forgot to answer, thanks for the review & merge:)

@steveoandrey steveoandrey deleted the feature/focus_in_dialog branch February 28, 2022 17:56
@@ -103,6 +103,14 @@ public bool IsVisible
/// </summary>
[Parameter] public EventCallback<bool> IsVisibleChanged { get; set; }


/// <summary>
Copy link

@ghost ghost Mar 21, 2023

Choose a reason for hiding this comment

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

API documentation missing, the summary explains the TitleContent RenderFragment. @steveoandrey

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accidental break API change API that needs approval enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants