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

Add editor onFocusChanged event to update active editor when switching editors #9013

Merged
merged 1 commit into from
Feb 9, 2021

Conversation

odeliat
Copy link
Contributor

@odeliat odeliat commented Feb 1, 2021

Signed-off-by: odelia odelia.zehavi@sap.com

What it does

Fixes: #9042
Relates to: #8921

When we split 2 editors side by side and switch between them leaving the cursor in the same spot the focus doesn't update.
On the editor-widget.ts we have an event onSelectionChanged but when the cursor doesn't change it won't fire

packages\editor\src\browser\editor-widget.ts:

this.toDispose.push(this.editor.onSelectionChanged(() => {
            if (this.editor.isFocused()) {
                this.selectionService.selection = this.editor;
            }
        }));

I added the onFocusChanged event to update this.selectionService.selection also on focus change.

How to test

I used the extension from the bug report.
The extension add a button on the editor/title when the file is .json
We need to change the when statement in the package.json to show the button only on files that under the page folder.

  1. Clone https://github.com/avishaik/vscode-editor-btn-bug
  2. update the when statement in the package.json in contributes -> menus -> editor/title to:

"resource =~ /^(.)/pages/(.)$/ && editorLangId == json && editorFocus == true"

  1. Build it npm run package and add it as extension to theia
  2. Create a .json file on the root folder
  3. Create a 'pages' folder and create another .json file in this folder
  4. Open both files side by side
  5. When pressing on the JSON file tab under the pages folder, the button shows only in this tab
  6. When pressing on the root JSON file tab, the button disappears from the .json file

Review checklist

Reminder for reviewers

Signed-off-by: odelia <odelia.zehavi@sap.com>
@colin-grant-work
Copy link
Contributor

colin-grant-work commented Feb 3, 2021

My testing reveals the following behavior:

  1. JSON preview editor widget, no icon:
    image

  2. JSON editor widget, icon appears:
    image

  3. JSON editor widget focused, other editor present, both get the icon:
    image

  4. Other editor focused, JSON editor widget present, neither gets icon:
    image

I believe 1. and 3. are incorrect, and 4. may or may not be desirable - the other icons appear regardless of whether the editor is focused - but @avishaik will need to weigh in on whether the intended behavior is that the icon should only appear when the editor is focused, or any time a JSON editor is open and visible.

@odeliat
Copy link
Contributor Author

odeliat commented Feb 4, 2021

@colin-grant-work, did you update the when statement in the package.json before creating the extension?
The desire behavior is to show the icon only when you are focus on the .json file.
On your testing -
1 - As I understand it was not the editor.
2 + 4 - This is correct as indented.
3 - This is incorrect - the when statement should be as described.

@odeliat odeliat closed this Feb 4, 2021
@odeliat odeliat reopened this Feb 4, 2021
@colin-grant-work
Copy link
Contributor

colin-grant-work commented Feb 4, 2021

@odeliat, Thanks for the reminder, I hadn't updated the when, and that does improve things. However, point (1) above still stands. In VSCode, the icon appears for a preview of the JSON (italic title):

image

In Theia, we still get no toolbar icon in that case:

image

I'm not sure it's desirable to require that plugins that might be used in Theia add && editorFocus == true to their when statements in order to achieve the same behavior as VSCode shows without that extra stipulation. Can you see any way to get VSCode's behavior without that rule?

@odeliat
Copy link
Contributor Author

odeliat commented Feb 7, 2021

@colin-grant-work The VSCode behavior is different than Theia.
On VSCode the editor icons appears only when the editor is focus - Theia display the icons always (unless it's editorFocus == true). We can open another issue in order to be in line with the VSCode behavior.

My code suggestion is to trigger the active editor to the correct one when the cursor is in the same position.

@colin-grant-work
Copy link
Contributor

colin-grant-work commented Feb 8, 2021

@odeliat, I agree your code fixes the specific problem of failure to update the focused editor, so I'm fine merging this as long as we track the remaining cases where Theia's behavior deviates from VSCode in handling editor-related when clauses. I've created a new issue (#9042) that describes the bug fixed here. Let's use this PR to close that issue, and leave #8921, which contains a lot of useful information about how the ContextKeyService is failing to perform the correct matches, to be fixed by later work.

@amiramw amiramw merged commit 05f5682 into eclipse-theia:master Feb 9, 2021
@paul-marechal paul-marechal added this to the 1.11.0 milestone Feb 25, 2021
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.

Focused Editor Context Not Updated When New Editor Focused
4 participants