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.

Sign up for GitHub

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

[editor] Show languages and focus file-tree while preview #7258

Closed
wants to merge 1 commit into from
Closed

[editor] Show languages and focus file-tree while preview #7258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/core/src/browser/widget-open-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,14 @@ export abstract class WidgetOpenHandler<W extends BaseWidget> implements OpenHan
await this.shell.activateWidget(widget.id);
} else if (op.mode === 'reveal') {
await this.shell.revealWidget(widget.id);
this.shell.currentChanged.emit({
Copy link
Member

Choose a reason for hiding this comment

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

only shell can fire such events via the widget tracker

Copy link
Member

Choose a reason for hiding this comment

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

Also it is bogus, revealing the widget does not change the focus and should not change the current widget

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review, do you have any suggestions as to how will we be able to fix this issue?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I still don't understand the issue: #7170 (comment). As I've mentioned already the editor preview should not be focused. It is correct.

cc @AlexTugarev @vince-fugnitto

Copy link
Member

@paul-marechal paul-marechal Mar 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@akosyakov try to preview a file in VS Code, even though the focus is not set to the opened preview, the status bar will correctly show the current language and information for said editor.

Right now in Theia the status bar uses shell.currentWidget to get its information from, and it seems to be driven by the FocusTracker instance, except we are not sure if we should register preview editors with it?

Copy link
Member

Choose a reason for hiding this comment

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

Understood, I don't think we can change the semantic of shell.currentWidget but I wonder whether we can build on top if a different semantic for a current widget of some kind: #7170 (comment)

// eslint-disable-next-line no-null/no-null
oldValue: this.shell.currentWidget || null,

// eslint-disable-next-line no-null/no-null
newValue: widget || null,

});
}
}

Expand Down
11 changes: 6 additions & 5 deletions packages/editor/src/browser/editor-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import { injectable, postConstruct, inject } from 'inversify';
import URI from '@theia/core/lib/common/uri';
import { RecursivePartial, Emitter, Event } from '@theia/core/lib/common';
import { WidgetOpenerOptions, NavigatableWidgetOpenHandler } from '@theia/core/lib/browser';
import { WidgetOpenerOptions, NavigatableWidgetOpenHandler, Widget } from '@theia/core/lib/browser';
import { EditorWidget } from './editor-widget';
import { Range, Position, Location } from './editor';
import { EditorWidgetFactory } from './editor-widget-factory';
Expand Down Expand Up @@ -51,8 +51,9 @@ export class EditorManager extends NavigatableWidgetOpenHandler<EditorWidget> {
protected init(): void {
super.init();
this.shell.activeChanged.connect(() => this.updateActiveEditor());
this.shell.currentChanged.connect(() => this.updateCurrentEditor());
this.onCreated(widget => widget.disposed.connect(() => this.updateCurrentEditor()));
this.shell.currentChanged.connect((sender, args) => this.updateCurrentEditor(args));
// eslint-disable-next-line no-null/no-null
this.onCreated(widget => widget.disposed.connect((sender, args) => this.updateCurrentEditor({ oldValue: null, newValue: null })));
}

protected _activeEditor: EditorWidget | undefined;
Expand Down Expand Up @@ -88,8 +89,8 @@ export class EditorManager extends NavigatableWidgetOpenHandler<EditorWidget> {
this.onCurrentEditorChangedEmitter.fire(this._currentEditor);
}
}
protected updateCurrentEditor(): void {
const widget = this.shell.currentWidget;
protected updateCurrentEditor(arg: { oldValue: Widget | null, newValue: Widget | null }): void {
const widget = arg.newValue ? arg.newValue : this.shell.currentWidget;
if (widget instanceof EditorWidget) {
this.setCurrentEditor(widget);
} else if (!this._currentEditor || !this._currentEditor.isVisible) {
Expand Down