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

Don't play with the focus when handling focus event #15408

Conversation

fcollonval
Copy link
Member

References

Fixes #15407

Code changes

Create a new protected method setMode that allow to turn off some side-effect related to focus
Turn off focus actions when changing mode in focus event handlers.

User-facing changes

In full windowing mode, we can switch to any cell even if the active cell is not in the view.

Backwards-incompatible changes

None

Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@fcollonval
Copy link
Member Author

fcollonval commented Nov 14, 2023

Need to test:

  • Switching between lab tabs
  • Switching between browser tab
  • Dealing with focus within an output

@krassowski krassowski added the bug label Nov 14, 2023
@fcollonval
Copy link
Member Author

js-notebook issue is legit:

  ● @jupyter/notebook › Notebook › #handleEvent() › focusout › should switch to command mode

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      1358 |           MessageLoop.sendMessage(widget, Widget.Msg.ActivateRequest);
      1359 |           expect(widget.mode).toBe('command');
    > 1360 |           expect(widget.activeCell!.editor!.hasFocus()).toBe(false);
           |                                                         ^
      1361 |         });
      1362 |
      1363 |         it('should set command mode', () => {

      at Object.<anonymous> (test/widget.spec.ts:1360:57)

@fcollonval fcollonval added this to the 4.0.x milestone Nov 16, 2023
@fcollonval fcollonval marked this pull request as ready for review November 16, 2023 13:51
@krassowski
Copy link
Member

Thanks! Conceptually and code-wise looks good. I will test locally tomorrow.
I think backport is not needed here and will change the milestone, but please correct me/revert milestone if I am wrong.

Copy link
Member

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

Thank you!

@krassowski krassowski merged commit 84d1f86 into jupyterlab:main Dec 1, 2023
77 of 81 checks passed
chisangajm3 added a commit to chisangajm3/jupyterlab that referenced this pull request Dec 18, 2023
* upstream/main: (7628 commits)
  Adopt ruff format (jupyterlab#15499)
  Bump scipy from 1.11.3 to 1.11.4 (jupyterlab#15474)
  [pre-commit.ci] pre-commit autoupdate (jupyterlab#15491)
  Pin `actions/labeler` to v4 to fix failing CI action (jupyterlab#15496)
  Add npm provenance issue to the list of release postmortems (jupyterlab#15493)
  Fix URLs in debugger-extension (jupyterlab#15462)
  Bump tj-actions/changed-files from 40.0.2 to 40.2.0 (jupyterlab#15471)
  Bump dessant/lock-threads from 4 to 5 (jupyterlab#15472)
  Bump pandas from 2.1.2 to 2.1.3 (jupyterlab#15473)
  Bump actions/github-script from 6 to 7 (jupyterlab#15470)
  Bump matplotlib from 3.7.2 to 3.8.2 (jupyterlab#15475)
  Bump rjsf to 5.13.4 (jupyterlab#15469)
  Don't play with the focus when handling focus event (jupyterlab#15408)
  [ci skip] Publish 4.1.0a4
  Updated light theme visited link colour to make text visible (jupyterlab#15406)
  Load custom CSS functionality and documentation (jupyterlab#14743)
  Added alt descriptions to a few icon and images (jupyterlab#15265)
  More robust galata/UI tests (jupyterlab#15355)
  Fix Shift + L not working in stdin (jupyterlab#15440)
  Upgrade releaser workflows for silent support (jupyterlab#15446)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scrolling jitter [windowed nb]
2 participants