Improve upon the accessibility and usage conventions of `Dialog` with the new `DialogV2` class which is developed as a companion to the ApplicationV2 infrastructure.

[typhonrt]

What happened?

The attempt to make Dialog act like a modal dialog when it is anything but seriously breaks focus traversal and provides a lot of other adverse behavior to occur. The offending code is Dialog._onKeyDown which was significantly altered for v10.

In handling the Tab key this is the particular offending code:

      event.preventDefault();
      event.stopPropagation();
      const dialogButtons = Array.from(document.querySelectorAll(".dialog-button"));
      const targetButton = event.shiftKey ? dialogButtons.pop() : dialogButtons.shift();
      targetButton.focus();

Let's consider this code as there are many issues.

• Because Dialog is not modal it is really bad to steal focus on any Tab press.

  • Consider a Dialog being rendered. Click on another app / window / actor sheet / input element that now appears above the Dialog. The user has no idea why tab order is broken. If you must keep this behavior invoke bringToTop; but please don't do just this as that is a hack on a hack and still is very problematic.

• Open two Dialog instances and here is where things get fun... Array.from(document.querySelectorAll(".dialog-button")); gets all dialog buttons rendered from all dialogs open. Tab will focus the first button found in the document flow out of all dialog buttons across all dialogs. Shift-Tab will focus the last button in the document flow across all dialogs open. Alternate between Tab and Shift-Tab and focus switches between dialog instances.

  • When two or more dialogs are open this behavior prevents normal tab order within a dialog sticking the focus as described above.

Solution: Remove the attempt at tab handling for Dialogs as they aren't modal. Consider actually implementing a modal dialog that uses a "glasspane" below the Dialog instance that visually obscures the screen and prevents any clicks onto other elements.

Here is a video showing broken functionality w/ two Dialog instances and how it affects my test app (w/ my UI framework, but any Foundry app is affected): v10-broken-dialog-focus.webm

What ways of accessing Foundry can you encounter this issue in?

• [X] Native App (Electron)
• [X] Chrome
• [X] Firefox
• [X] Safari
• [ ] Other

Reproduction Steps

^

What core version are you reporting this for?

Version 10.291

Relevant log output

No response

Bug Checklist

• [X] The issue occurs while all Modules are disabled

[aaclayton]

It sounds like you don't agree, but the intended usage of our Dialog class is that it does take over the user's focus and become the prioritized app that the user is interacting with. We'll look into ways to improve upon the problems you mention, however.

[typhonrt]

There is a large gap between intended usage and present implementation. I disagree with the current usability and implementation as it is broken. I do agree that dialogs are useful; particularly modal dialogs. I have an extended dialog implementation in my UI framework similar to Dialog and data compatible, but fix all of the edge cases present in the core Dialog implementation and add a bit more usability aspects. I also provide a true modal dialog option that blocks input except for the dialog. I can certainly show examples of this interaction if useful in any discussion. I caught this issue w/ Dialog because I'm doing extensive a11y / keyboard navigation testing and throw in plenty of core / mixed window scenarios.

General suggestions:

• Dialogs by default should always appear over the top of all other applications and can't be obscured by normal applications.
• For all app windows including dialogs "focus cycling / trapping" where tab order cycles around to the top of the app window or via Shift-tab the last focusable element in the app window is rather handy. Admittedly, that will be harder for core to implement w/ App v1 vs the options I have at my disposal. What good is it if the intended usage is to take over the user's focus, but one can simply tab navigate out of the dialog to the next focusable element in the document flow. If a dialog is at the end of the document flow which it is naturally when being the last window added to document.body / tab order continues on to the browser controls itself.
• If the intended usage is taking over the user's focus make a true modal dialog option. Normal dialogs similar to the current situation are fine too without the focus stealing.

In the short term I recommend backing out the tab / focus stealing. In the medium term implementing a true modal solution w/ additional option (IE pass modal: true in Dialog options) is pertinent + well tested "focus trapping" is the way forward.

I have implemented in my own UI framework everything I'm talking about, so it's not just conjecture on my side for what may or may not work well.

[aaclayton]

Need to double check, this may have already been fixed by the focus changes shipped in Testing 1

[typhonrt]

Need to double check, this may have already been fixed by the focus changes shipped in Testing 1

I'll give things a look today and report back.

[cswendrowski]

@typhonrt is this still an issue?

[typhonrt]

I've been deep in it.. I'll absolutely bump this to the top next session, so I'll have a response tomorrow.

[typhonrt]

So, I'm still seeing the same behavior essentially / not much changed. This test was done on v11 Build 297. When multiple core Dialogs are open they interfere with each other. More or less the same symptoms as before. When one or more Dialogs are open they completely steal focus traversal from any other elements loaded. They don't seem to affect my TJSDialog UI traversal handling, but the core dialog affect normal focus traversal otherwise.

My original synopsis is still valid insofar that perhaps in v12 you guys can look into implementing a modal Dialog. A modal dialog is where you can trap focus. The attempt to alter focus traversal for any normal Dialog is perilous at best. I still recommend backing out the focus capturing code for normal core Dialogs. It clearly breaks even when just two core Dialogs are open.

Here is a breakdown of the attached demo video.

• I show the behavior of multiple core dialogs open
• I open an app that has focus traversal (core dialogs steal focus)
• I then open a modal TJSDialog from my framework (focus is not stolen; this is due to my dialog focus management)
• I close the TJSDialog and show that as long as any core Dialog is on screen focus traversal is borked.
• I show what happens when only one core Dialog is on screen.
• I then show that when no core Dialogs are open that focus traversal returns to normal.

Low quality demo video (10MB limit):

https://github.com/foundryvtt/foundryvtt/assets/311473/c25594e3-465d-423e-a89c-2e038f3a3280

[aaclayton]

@typhonrt appreciate the feedback. discussed with the team and we are going to hold this for V12 when we work on application design and focus on implementing a more proper "Dialog" implementation at that time.

[typhonrt]

Cool.. I'd be glad to chat briefly about all that and share some insights that are relevant after you guys get a break from the v11 launch. I can definitely prepare a 1 page synopsis of relevant things. There is a really easy performance gain for the App v1 framework that actually removes some code.

[mouse0270]

Just wanted to throw my hat in here, mostly because I want pinged when updates to this happens. But I would like to agree that though the intention is for users to interact with the Dialog and move on makes sense, is completely countered by the fact that you allow module developers to use the Dialog, because unless you get every single developer to check if a dialog is already opened before you open yours, then just the fact people install more than one module completely breaks this intended function.

I hope that when looking at v12 implantation of this, you either reconsider the desired intention of the dialog class, or force users to actually interact with them one at a time, or handle interacting with dialogs better, or provide some sort of interface for queueing dialogs to prevent these issues as currently this UX is a little terrible.

Though in fairness, this is kinda of an edge case as I only happened to notice this when I triggered a dialog for my module and another module happened to trigger theirs. So I suspect its not extremely common as most people probably just create their own dialogs using the Application or FormApplication.

Thanks for all your hard work.

[aaclayton]

We should work to resolve some of these issues with the DialogV2 that is under development as a companion to the ApplicationV2 infrastructure.