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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notebook: going back to notebook does not restore focus to active cell #100458

Closed
bpasero opened this issue Jun 18, 2020 · 2 comments
Closed

Notebook: going back to notebook does not restore focus to active cell #100458

bpasero opened this issue Jun 18, 2020 · 2 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders notebook verified Verification succeeded
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Jun 18, 2020

Steps to Reproduce:

  1. open a notebook
  2. click into a cell
  3. click on another editor tab
  4. click back into notebook

=> 馃悰 focus does not return into the cell

@bpasero bpasero added bug Issue identified by VS Code Team member as probable bug notebook labels Jun 18, 2020
@rebornix rebornix added this to the June 2020 milestone Jun 24, 2020
rebornix added a commit that referenced this issue Jun 24, 2020
@rebornix
Copy link
Member

I pushed a change to narrow the problem. I remembered vividly that almost 2 or 3 years ago @bpasero reported a similar issue with the same root cause. What happens is

  • Mouse down on title bar item
    • open notebook editor
    • we restore view state, focus the editor in the cell
  • Mouse up
    • the active element becomes tab control again, triggers a blur event
    • we set the cell focus mode to container (which means it should focus the container other than the inner editor) Even untitled files are not dirty until you start typing.

I think @roblourens made it work pretty well but not sure when it broke and how ...

rebornix added a commit that referenced this issue Jun 24, 2020
@roblourens
Copy link
Member

A few things going on, so we do listen on onDidBlurEditorWidget and fix the cell's focusMode when the editor blurs. This doesn't take effect here because the FocusTracker delays firing its blur event with setTimeout, so it doesn't come until after the editor has had focus() called on it, so at that point, we think the cell is focused. But changing that would actually not be the right fix, because if we set focusMode to container, then we will not try to put focus back in the editor when the notebook editor gets focus. focusMode needs to be whether the cell editor is 'supposed to' have focus, not whether it actually does. So for now I'll just fix this by removing the check for whether focusMode actually changed before we fire the change event.

Maybe I should also change CodeCell/MarkdownCell's updateFocusMode to also check where focus went after onDidBlurEditorWidget. If it's inside the notebook editor, we should set focusMode to Container. If it's outside the notebook editor, it should stay as Editor. Also, maybe it would make sense for that to be a property on the editor instead of individual cells.

@bpasero bpasero added the verified Verification succeeded label Jun 30, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Aug 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders notebook verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants
@roblourens @rebornix @bpasero and others