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

Introduce "Active Element" concept #14879

Open
tsmaeder opened this issue Feb 10, 2025 · 1 comment
Open

Introduce "Active Element" concept #14879

tsmaeder opened this issue Feb 10, 2025 · 1 comment

Comments

@tsmaeder
Copy link
Contributor

Feature Description:

Right now, many decisions, in particular menu enablement, are based on the focused element. If we talk of "focused element", one needs to understand that "focus" always means "keyboard focus". This leads to problems like this one: #14002. Without going into detail: the problem there is that the menu bar accepts the focus (it needs, to, otherwise it would not be navigable by keyboard and therefore not accessible). This leads to the menu being rebuilt. We also have some workarounds in the code because Theia needs the focus on the "right" element to compute the enabled and visible elements. This code is brittle and prone to break when changing anything (like making dynamic menu contributions actually work).

I propose the we introduce the concept of "active element" per window and base action enablement upon this concept (i.e. the context keys get set when the active element changes). The Eclipse IDE has the similar concept of "WorkbenchPart", which can be either a view or an editor. In Theia, we'd have to analyze which elements constitute the parts, probably editors and view parts?

A workbench part generally becomes the active part through different routes:

  1. An UI element inside the part accepts focus
  2. The Window in which the part resides becomes the active window (for example because the use opens the window) and the part was the last or default active part for this window
  3. The part is opened
  4. The active part is changed through some keybinding or programmatic means (think "the debugger stopped on a breakpoint and now the debugger should become the active part")

When the keyboard focus moves out of the active part, the part remains the active part until another part becomes the active part. So things like the toolbar and menus can never be the active part and influcence action enablement.

Benefits

We could get rid of the hacks in our menus and streamline the action enablement code. I just spent two days analyzing an issue in our menu because we're overriding stuff from lumino/phosphor in complicate ways.

@sdirix
Copy link
Member

sdirix commented Feb 10, 2025

@tsmaeder Thanks for opening the issue.

Let me emphasize that the current focus-based system is inherently broken. As focus is a specific concept in the Browser with all kind of associated behavior, reusing it for the "activeElement" management leads to:

  • a lot of hacky code which easily breaks
  • weird observable user issues like elements gaining/losing focus without user interaction
  • usability issues as we have a lot of focus manipulation, breaking keyboard navigation, screen readers etc.

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

No branches or pull requests

2 participants