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

Application shell widgets must take focus on activation #1061

Closed
spoenemann opened this issue Jan 16, 2018 · 2 comments · Fixed by #1604
Closed

Application shell widgets must take focus on activation #1061

spoenemann opened this issue Jan 16, 2018 · 2 comments · Fixed by #1604

Comments

@spoenemann
Copy link
Contributor

When a widget is activated, its onActivateRequest method is invoked. However, the default implementation does nothing! For widgets that are added directly to the application shell, this is a problem, because ApplicationShell tracks the active widget through a FocusTracker, which listens to focus changes. So if a widget that is added to the shell does not implement onActivateRequest, it will never be seen as active. This causes peculiar side effects; for example the tab bar context menu commands operate on the current (last active) widget, so doing right-click and Close on a widget's tab can cause the wrong tab to close (the last one that took focus instead of the one that was right-clicked).

I fixed this problem for the already implemented views in #1028, but it remains an issue for every new Theia widget. It is not obvious for widget implementers that they need to take care of the focus.

@spoenemann
Copy link
Contributor Author

One possible solution could be to create a common abstract superclass for all application shell widgets that has some default behavior regarding focus, e.g.

    protected onActivateRequest(msg: Message): void {
        this.node.focus();
    }

However, this works only if the node of the widget can take focus. HTML elements that do not have any default user interaction, such as div, are focusable only if the tabindex attribute is set.

@svenefftinge
Copy link
Contributor

Instead of solving it generically, which seems to be hard. We could maybe improve the error reporting for widgets, that didn't get focus.

spoenemann added a commit that referenced this issue Mar 29, 2018
…es not accept focus

Signed-off-by: Miro Spönemann <miro.spoenemann@typefox.io>
spoenemann added a commit that referenced this issue Mar 29, 2018
…es not accept focus

Signed-off-by: Miro Spönemann <miro.spoenemann@typefox.io>
spoenemann added a commit that referenced this issue Apr 3, 2018
…es not accept focus

Signed-off-by: Miro Spönemann <miro.spoenemann@typefox.io>
spoenemann added a commit that referenced this issue Apr 10, 2018
…es not accept focus

Signed-off-by: Miro Spönemann <miro.spoenemann@typefox.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants