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

Change "Interface" menu to "Open in...", shorten names of menu items inside #6847

Merged
merged 18 commits into from
May 5, 2023

Conversation

andrii-i
Copy link
Contributor

@andrii-i andrii-i commented Apr 19, 2023

Without this change:
image

With this change:
Screenshot 2023-04-19 at 9 52 47 PM
Screenshot 2023-04-19 at 9 58 14 PM
image

@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch andrii-i/notebook/open_with_menu

@andrii-i andrii-i changed the title Add option to open with nbclassic Change "Interface" menu to "Open with...", adjust command names inside, add conditional option to open with nbclassic Apr 20, 2023
@andrii-i andrii-i changed the title Change "Interface" menu to "Open with...", adjust command names inside, add conditional option to open with nbclassic Change "Interface" menu to "Open with...", adjust command names, add conditional option to open with nbclassic Apr 20, 2023
@andrii-i andrii-i changed the title Change "Interface" menu to "Open with...", adjust command names, add conditional option to open with nbclassic Change "Interface" menu to "Open in...", shorten command names inside, add conditional option to open with nbclassic Apr 20, 2023
@andrii-i andrii-i changed the title Change "Interface" menu to "Open in...", shorten command names inside, add conditional option to open with nbclassic Change "Interface" menu to "Open in...", shorten names of menu items inside, add conditional option to open with nbclassic Apr 20, 2023
@jtpio jtpio added this to the 7.0 milestone Apr 20, 2023
@jtpio
Copy link
Member

jtpio commented Apr 20, 2023

Thanks @andrii-i for looking into this!

@andrii-i
Copy link
Contributor Author

andrii-i commented Apr 20, 2023

Thank you for surfacing need to preserve longer wording in command palette @jtpio, it's important not to erode usability. I'm still working on adding conditional option to open in nbclassic if nbclassic is installed but current state of this PR is self-contained and complete for task 1. In case Notebook 7 release would be to happen today, this can be merged as-is

@andrii-i andrii-i changed the title Change "Interface" menu to "Open in...", shorten names of menu items inside, add conditional option to open with nbclassic Change "Interface" menu to "Open in...", shorten names of menu items inside May 2, 2023
@andrii-i andrii-i marked this pull request as ready for review May 2, 2023 22:53
@andrii-i
Copy link
Contributor Author

andrii-i commented May 2, 2023

Dynamically detecting nbclassic would require server modification so I will take care of it in a separate PR.

@andrii-i
Copy link
Contributor Author

andrii-i commented May 2, 2023

Bot please update playwright snapshots

@jtpio
Copy link
Member

jtpio commented May 3, 2023

Ah looks like the bot might be broken, but not sure why: https://github.com/jupyter/notebook/actions/runs/4866690782/jobs/8678479167

@jtpio
Copy link
Member

jtpio commented May 4, 2023

cc @yuvipanda for awareness

@jtpio
Copy link
Member

jtpio commented May 4, 2023

@andrii-i actually one of the snapshot failures is relevant. The menu entry should be like the command palette and say "Open in JupyterLab":

image

@andrii-i andrii-i requested a review from jtpio May 5, 2023 01:15
@andrii-i
Copy link
Contributor Author

andrii-i commented May 5, 2023

@jtpio managed to resolve all conflicts, it's all green now 🟢. Manually finding and replacing snapshots is definitely not very optimal and takes time.

@jtpio
Copy link
Member

jtpio commented May 5, 2023

FYI @andrii-i I pushed a small change to avoid the use of an extra command to handle different labels.

Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

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

Thanks!

@jtpio jtpio merged commit d7d110c into jupyter:main May 5, 2023
@jtpio jtpio mentioned this pull request May 5, 2023
2 tasks
@andrii-i
Copy link
Contributor Author

andrii-i commented May 5, 2023

Thank you @jtpio

@andrii-i andrii-i deleted the open_with_menu branch May 5, 2023 17:28
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shorten "Interface" menu and options
2 participants