Added fullscreen feature with command alt f #16308

dhysdrn · 2024-05-08T15:21:04Z

References:

Fixes #8710 - Add fullscreen option to JupyterLab
Adding the base fullscreen mode functionality in preparation for fullscreen mode expansion to allow individual widgets to enter fullscreen mode for presentation purposes per the requests in PR #10676.

Code Changes:

jupyterlab/packages/application-extension/src/index.tsx
Added Lines 113-114 - Export the toggleFullscreenMode component.
Added Lines 438-454 - The toggleFullscreenMode command component
Added Line 544 - Register the new command as a CommandIDs entry in the palette
jupyterlab/packages/application-extension/schema/commands.json
Added Lines 293-296 - Include keyboard shortcut of Alt + F
Added Lines 50-54 - Include the fullscreen option in the top bar menu
jupyterlab/packages/application/shell.d.ts
Added Lines 282-287 Created the entry to get/set the fullscreenMode css value to the shell div (preparation for individual fullscreen widgets)

User-Facing Changes:

Provides a menu option to enter fullscreen mode and uses Alt + F as the shortcut key to toggle into and out of fullscreen mode.

F11 remains disabled as a means to enter fullscreen mode to maintain current production JupyterLab behavior.

If entering fullscreen using the Alt + F shortcut or button in the view > Fullscreen Mode menu, users can exit with the view > Fullscreen Mode menu option, Alt + F, ESC key, or F11 key, and JupyterLab maintains fullscreen synchronization of the menu item, check box, and CSS styling.

Backwards-incompatible Changes:

None

jupyterlab-probot · 2024-05-08T15:21:06Z

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

welcome · 2024-05-08T15:21:07Z

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.

You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

Rob-P-Smith · 2024-05-08T15:23:52Z

@meeseeksdev tag enhancement

bot please update snapshots

packages/application-extension/src/index.tsx

github-actions · 2024-05-08T15:40:04Z

Documentation snapshots updated.

github-actions · 2024-05-08T16:07:58Z

Galata snapshots updated.

packages/application-extension/src/index.tsx

Rob-P-Smith · 2024-05-13T22:30:02Z

@krassowski We've adjusted the code to align more with the existing codebase and cleaned up the uncaught error issue that previously failed "jplm run eslint:typed".

If this is acceptable, we'd like to close this and work on full-screening individual elements as a separate issue next, e.g. terminal/notebook/widgets etc as requested in the previous PR that attempted to solve issue #8710 here:
#10676

bot please update snapshots

github-actions · 2024-05-13T22:43:14Z

Documentation snapshots updated.

github-actions · 2024-05-13T23:10:25Z

Galata snapshots updated.

krassowski · 2024-05-16T16:35:27Z

Just as FYI, you do not need to write down the code changes by file and line range in the PR description (these these quickly get outdated). What I would recommend is to focus on listing major API changes if there are any, e.g. that fullScreenMode was added to LabShell.

krassowski

I do not have a strong opinion on this, but I think it is fine to start with.
Before this can be merged here is a few suggestions.

packages/application-extension/src/index.tsx

...ntation/debugger.test.ts-snapshots/debugger-stop-on-raised-exception-documentation-linux.png

galata/test/jupyterlab/notebook-run.test.ts-snapshots/notebook-panel-1-jupyterlab-linux.png

krassowski · 2024-05-16T16:43:08Z

packages/application/src/shell.ts

+  /**
+   * Toggle the fullscreen mode based on user inputs
+   */
+  setFullscreenMode(): void {


Why do we need this method? Could it be handled in the setter of fullscreenMode property instead to minimize the API surface?

krassowski · 2024-05-16T16:47:07Z

packages/application/src/shell.ts

+   * `jp-mod-fullscreenMode` CSS class.
+   */
+  get fullscreenMode(): boolean {
+    return this.hasClass('jp-mod-fullscreenMode');


I don't think we should store state which can be modified by the JS API by using a CSS class - this can lead to inconsistent state if an extension or JS in outputs of user executed code manipulate the fullscreen state (for example rise extension for presenting slides could do that).

I would prefer if the getter returned the state derived from the browser API like fullscreenElement. That said, I am not even sure if we need a getter/setter here - if we are not setting the CSS class, then we could get away without extending the public API of LabShell at all, which will make merging this PR much easier.

We made the changes and removed the getter/setter and directly queried the fullscreenElement value, we also reverted toggle sidebar widget to its original code. Thank you! (:

dhysdrn · 2024-05-20T21:46:03Z

@krassowski it looks like we're failing some tests and unsure what we need to do to fix it, could you help us with this?

krassowski · 2024-05-21T08:27:11Z

Some of the failing tests are just flaky, I can restart these. One screenshot test appears to need updating:

[jupyterlab] › test/jupyterlab/menus.test.ts:30:9 › General Tests › Open menu item View>Appearance

bot please update galata snapshots (we may need to revert some spurious updates afterwards)

packages/application/src/shell.ts

github-actions · 2024-05-21T09:06:59Z

Galata snapshots updated.

galata/test/jupyterlab/notebook-run.test.ts-snapshots/notebook-panel-1-jupyterlab-linux.png

galata/test/jupyterlab/notebook-edit.test.ts-snapshots/split-cell-jupyterlab-linux.png

galata/test/jupyterlab/menus.test.ts-snapshots/opened-menu-view-appearance-jupyterlab-linux.png

dhysdrn · 2024-05-26T18:10:21Z

bot please update galata snapshots

github-actions · 2024-05-26T18:23:24Z

Documentation snapshots updated.

github-actions · 2024-05-26T18:26:53Z

Galata snapshots updated.

dhysdrn · 2024-05-27T16:06:43Z

@krassowski i think we made all the right changes, could you help us with this?

krassowski · 2024-05-27T17:51:10Z

packages/application/src/shell.ts

+  /**
+   * Toggle the fullscreen mode based on user inputs
+   */
+  toggleFullscreenMode(): void {


Can I ask for one more thing - to move this method into a function in packages/application-extension/src/index.tsx?

The rationale is a follows: once we add something to LabShell class , any attempt to change it will be potentially breaking for an extension. Since there is further expansion of the fullscreen capabilities requested (to cover making individual elements take the full screen) enhancements, I think it is likely that the implementation may change.

@krassowski we’ve removed the method and added it to packages/application-extension/src/index.tsx, let us know if it looks good

….tsx

dhysdrn · 2024-06-06T21:12:59Z

@krassowski Hey, just checking in 👋🏻 We would love to close this out and get it merged. Do let us know if we’re good or if we need anymore changes (:

krassowski

Testing:

GNOME Web (webkit): works ok
Chrome: works ok
Firefox: works when clicking on the menu, does not work with Alt + F - it throws:

Request for fullscreen was denied because Element.requestFullscreen() was not called from inside a short running user-generated event handler. [index.tsx:458:37](webpack://jupyterlab/packages/application-extension/src/index.tsx)
Failed to enter fullscreen mode. TypeError: Fullscreen request denied
    execute index.tsx:458
    execute index.es6.js:365
    _executeKeyBinding index.es6.js:569
    processKeydownEvent index.es6.js:473
    evtKeydown lab.ts:217

In both Chrome and Firefox, entering full screen manually with F11 and then invoking the application:toggle-fullscreen-mode command forces the user to escape the full screen twice. This is odd.

krassowski · 2024-06-07T08:35:26Z

packages/application-extension/schema/commands.json

@@ -47,6 +47,11 @@
      "keys": [""],
      "selector": "body"
    },
+    {
+      "command": "application:toggle-fullscreen-mode",
+      "keys": ["Alt F"],


Alt + F is used by browsers to:

open File menu (Firefox)

open three dots menu (Chrome/Edge)

It may or may not be ok to override these. We can discuss this on JupyterLab call on Wednesday. Would it be also ok to just use F11 which is typically default for full screen? This way the contribution of this PR is:

it adds a menu entry

it allows user to change the shortcut

it does not force a no-standard shortcut which is normally used for something else

Added fullscreen feature with command alt f

b664e90

github-actions bot assigned dhysdrn May 8, 2024

github-actions bot added the pkg:application label May 8, 2024

lumberbot-app bot added the enhancement label May 8, 2024

github-advanced-security bot found potential problems May 8, 2024

View reviewed changes

packages/application-extension/src/index.tsx Fixed Show resolved Hide resolved

Updated fullscreen feature

b3c0c62

dhysdrn force-pushed the main branch from dcc517e to b3c0c62 Compare May 8, 2024 16:15

github-advanced-security bot found potential problems May 8, 2024

View reviewed changes

packages/application-extension/src/index.tsx Fixed Show resolved Hide resolved

krassowski added this to the 4.3.0 milestone May 9, 2024

Ran prettier to fix lint issue

4b62b2d

dhysdrn force-pushed the main branch from 3c123bc to 4b62b2d Compare May 10, 2024 01:21

github-advanced-security bot found potential problems May 10, 2024

View reviewed changes

packages/application-extension/src/index.tsx Fixed Show fixed Hide fixed

Fixed errors

04dd407

krassowski requested changes May 16, 2024

View reviewed changes

Fixed issues based on feedback from last submission

4eccaa3

dhysdrn force-pushed the main branch from 381a9a4 to 4eccaa3 Compare May 16, 2024 21:31

dhysdrn added 2 commits May 16, 2024 14:39

Removed getter and setter for fullscreen mode

b8ca056

Ran prettier to fix format

c522cba

krassowski reviewed May 21, 2024

View reviewed changes

packages/application/src/shell.ts Outdated Show resolved Hide resolved

krassowski reviewed May 21, 2024

View reviewed changes

galata/test/jupyterlab/notebook-run.test.ts-snapshots/notebook-panel-1-jupyterlab-linux.png Outdated Show resolved Hide resolved

krassowski reviewed May 21, 2024

View reviewed changes

galata/test/jupyterlab/notebook-edit.test.ts-snapshots/split-cell-jupyterlab-linux.png Outdated Show resolved Hide resolved

krassowski reviewed May 21, 2024

View reviewed changes

galata/test/jupyterlab/menus.test.ts-snapshots/opened-menu-view-appearance-jupyterlab-linux.png Outdated Show resolved Hide resolved

Changed setFullscreen to toggleFullscreen

90f6e0a

dhysdrn force-pushed the main branch from 5ca6ac8 to 90f6e0a Compare May 21, 2024 21:02

Update Playwright Snapshots

a9ba31c

Update Playwright Snapshots

0fac91c

Revert snapshot updates

860f5a4

github-actions bot added the tag:Testing label May 27, 2024

krassowski reviewed May 27, 2024

View reviewed changes

Removed toggleFullscreenMode from shell and added function into index…

649ffa9

….tsx

krassowski reviewed Jun 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added fullscreen feature with command alt f #16308

Added fullscreen feature with command alt f #16308

dhysdrn commented May 8, 2024 •

edited by JasonWeill

jupyterlab-probot bot commented May 8, 2024

welcome bot commented May 8, 2024

Rob-P-Smith commented May 8, 2024 •

edited

github-actions bot commented May 8, 2024

github-actions bot commented May 8, 2024

Rob-P-Smith commented May 13, 2024

github-actions bot commented May 13, 2024

github-actions bot commented May 13, 2024

krassowski commented May 16, 2024

krassowski left a comment •

edited

krassowski May 16, 2024

krassowski May 16, 2024

dhysdrn May 16, 2024

dhysdrn commented May 20, 2024

krassowski commented May 21, 2024

github-actions bot commented May 21, 2024

dhysdrn commented May 26, 2024 •

edited

github-actions bot commented May 26, 2024

github-actions bot commented May 26, 2024

dhysdrn commented May 27, 2024

krassowski May 27, 2024

dhysdrn May 28, 2024

dhysdrn commented Jun 6, 2024

krassowski left a comment

krassowski Jun 7, 2024

Added fullscreen feature with command alt f #16308

Are you sure you want to change the base?

Added fullscreen feature with command alt f #16308

Conversation

dhysdrn commented May 8, 2024 • edited by JasonWeill

References:

Code Changes:

User-Facing Changes:

Backwards-incompatible Changes:

jupyterlab-probot bot commented May 8, 2024

welcome bot commented May 8, 2024

Rob-P-Smith commented May 8, 2024 • edited

github-actions bot commented May 8, 2024

github-actions bot commented May 8, 2024

Rob-P-Smith commented May 13, 2024

github-actions bot commented May 13, 2024

github-actions bot commented May 13, 2024

krassowski commented May 16, 2024

krassowski left a comment • edited

Choose a reason for hiding this comment

krassowski May 16, 2024

Choose a reason for hiding this comment

krassowski May 16, 2024

Choose a reason for hiding this comment

dhysdrn May 16, 2024

Choose a reason for hiding this comment

dhysdrn commented May 20, 2024

krassowski commented May 21, 2024

github-actions bot commented May 21, 2024

dhysdrn commented May 26, 2024 • edited

github-actions bot commented May 26, 2024

github-actions bot commented May 26, 2024

dhysdrn commented May 27, 2024

krassowski May 27, 2024

Choose a reason for hiding this comment

dhysdrn May 28, 2024

Choose a reason for hiding this comment

dhysdrn commented Jun 6, 2024

krassowski left a comment

Choose a reason for hiding this comment

krassowski Jun 7, 2024

Choose a reason for hiding this comment

dhysdrn commented May 8, 2024 •

edited by JasonWeill

Rob-P-Smith commented May 8, 2024 •

edited

krassowski left a comment •

edited

dhysdrn commented May 26, 2024 •

edited