Skip to content

Conversation

adamint
Copy link
Member

@adamint adamint commented Aug 29, 2025

Pull Request

📖 Description

Exposes two functions to get the focused element and set focus in dialog-utils.js. Uses that in the dialog provider to get the focused element before the dialog is created (which messes with focus), and then to reset focus after close. Dialog doesn't always close immediately, so I added a small delay.

Before:

Grabacion.de.pantalla.2025-08-29.a.la.s.1.05.00.a.m.mov

After:

Grabacion.de.pantalla.2025-08-29.a.la.s.1.06.55.a.m.mov

🎫 Issues

Fixes #4094

👩‍💻 Reviewer Notes

You can test this using the Dialog samples.

📑 Test Plan

✅ Checklist

General

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

  • I have added a new component
  • I have added Unit Tests for my new component
  • I have modified an existing component
  • I have validated the Unit Tests for an existing component

⏭ Next Steps

@MarvinKlein1508
Copy link
Collaborator

Just because I'm curious: what happens if I open the dialog via code somehow? Which element will be focused then?

For example in my case I open a modal directly in OnParameterSetAsync when a certain condition is invalid.

@vnbaaij
Copy link
Collaborator

vnbaaij commented Aug 29, 2025

I would say nothing happens then. I think this is done when the code from @dvoituron 's last review comment gets added

@adamint
Copy link
Member Author

adamint commented Aug 29, 2025

Just because I'm curious: what happens if I open the dialog via code somehow? Which element will be focused then?

For example in my case I open a modal directly in OnParameterSetAsync when a certain condition is invalid.

Whatever element that is currently focused at the time that OnParameterSetAsync is called. In my opinion, focus should always return back to the previous element so that users of screen readers can continue to navigate the page in order.

@adamint adamint requested a review from dvoituron August 29, 2025 21:13

return await Task.Run(async () =>
{
var previouslyFocusedElement = await _module.InvokeAsync<IJSObjectReference>("getActiveElement");
Copy link
Collaborator

Choose a reason for hiding this comment

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

You will find in the documentation: "If the focused element is within a document tree that's not descended from the current document (for example, the focused element is in the main document, and the invoking document is an embedded iframe), then this will be null."
https://developer.mozilla.org/en-US/docs/Web/API/Document/activeElement

So, previouslyFocusedElement can be null and you need to manage this case in DialogInstance and all methods called.

@@ -22,6 +25,8 @@ public DialogInstance(Type? type, DialogParameters parameters, object content)

public DialogParameters Parameters { get; internal set; }

internal IJSObjectReference PreviouslyFocusedElement { get; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

This property must be nullable. See my other comment

@dvoituron
Copy link
Collaborator

This new feature should be added to the “dev-v5” branch for the new Dialog component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: keyboard focus does not return to previous element after closing FluentDialog
4 participants