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

Reopen recently opened/closed files and modal navigation #15483

Merged
merged 39 commits into from
Mar 21, 2024

Conversation

krassowski
Copy link
Member

@krassowski krassowski commented Dec 4, 2023

References

Punchlist

  • galata tests
  • add an icon [↗] to sidebar

Code changes

Compared to jupyterlab-recents extension:

  • remove polling for validation of paths; this was generating many background requests and was deemed undesirable overhead if we were to increase the number of requests by a factor of 10 (even though these were lightweight GET requests)
  • do not send a get request to fetch the model/content type of the document for the that the user switch to - we already have it available at hand now thanks to integration directly into docmanager (this is not public/not emitted by the signal which is why the extension had to have this sub-optimal implementation)
  • path validation is now conducted when opening the sub-menu, and when clicking on the file, limiting any overhead to the users of the feature
  • when an item is discovered to be invalid after submenu was opened it will not be removed but greyed out to avoid jitter and user accidentally clicking a wrong item
  • split recents into "recently opened" and "recently closed"
  • use IFileBrowserCommands to explicitly depend on the availability of the filebrowser to open directories (and to fallback to document manager command if filebrowser does not provide such token

Because rather than adding a new package the extension gets integrated on the lower level by adding separate plugins to relevant existing packages (document manager, main menu) merging git history did no make much sense; instead the autorship was preserved by using Co-authored-by feature.

User-facing changes

Open Recent submenu

image

Recently closed in the sidebar

image

Also seen is the [↗] button to open modal

Reopen modal

reopen-widget-2

Backwards-incompatible changes

None.

File browsers are encouraged to implement IFileBrowserCommands which was previously a no-op; if they don't then the recents menu will not show the directories.

krassowski and others added 6 commits December 4, 2023 00:22
Co-authored-by: Frédéric Collonval <fcollonval@gmail.com>
Co-authored-by: Shreyas Cholia <shreyas@gmail.com>
Co-authored-by: Matt Henderson <mhenderson@lbl.gov>
Co-authored-by: Trevor Slaton <trevor.slaton@gmail.com>
Co-authored-by: Adrien Delsalle <ad.delsalle@gmail.com>
The cycle was introduced by removing void on returned Promise.
Because void operator returns undefined, the promise on
`IFileBrowserCommands` was never awaited (on main branch)
which meant that `@jupyterlab/filebrowser-extension:widget`
(the only dependant of commands token) was able to initialise
as soon as the commands were activated (not waiting for the
reveal/settings promises cascade); this commits restores
ignoring the result of the promise and instead returns
the command mapping after the promise was invoked.
Move type to `Private`, add docstrings

Automatic application of license header
@krassowski krassowski added this to the 4.2.0 milestone Dec 4, 2023
Copy link

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

@gabalafou
Copy link
Contributor

You've probably already thought through this, but just in case, I wanted to mention that in Chrome on my Mac, Command Shift A opens the browser's "Search Tabs" modal.

@krassowski
Copy link
Member Author

Any suggestions on alternative shortcuts we could use?

A part of me wishes that we had two sets of shortcuts: one for JupyterLab Desktop and one for when running in a standard browser.

@krassowski
Copy link
Member Author

For now switching to Ctrl + Alt + A as it is not used by major browsers/operating systems as per https://en.wikipedia.org/wiki/Table_of_keyboard_shortcuts

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @krassowski

I have an optional suggestion.

packages/docmanager/src/recents.ts Outdated Show resolved Hide resolved
Co-authored-by: Frédéric Collonval <fcollonval@gmail.com>
import { PageConfig } from '@jupyterlab/coreutils';
import { Contents, ServerConnection } from '@jupyterlab/services';
import { IStateDB } from '@jupyterlab/statedb';
import { IDisposable } from '@lumino/disposable';

Check notice

Code scanning / CodeQL

Unused variable, import, function or class

Unused import IDisposable.
@krassowski krassowski marked this pull request as draft March 20, 2024 19:16
@krassowski
Copy link
Member Author

Snapshots will need updating after merging. I think one of the docs snapshots needs some work in general.

@krassowski
Copy link
Member Author

krassowski commented Mar 21, 2024

but please update documentation snapshots (now, try again)

Before this commit we had two very similar snapshots:
- interface-tabs-documentation-linux.png
- running-layout-documentation-linux.png

both showing "open tabs" and "kernels" sections.
After this commit the former now only shows "open tabs"
reducing the burden reviewing snapshots when kernels
are flaky or when kernels section changes
Copy link
Contributor

Documentation snapshots updated.

Copy link
Contributor

Documentation snapshots updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants