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 sort context menu items by selector #10666

Merged
merged 1 commit into from Aug 13, 2021

Conversation

fcollonval
Copy link
Member

@fcollonval fcollonval commented Jul 21, 2021

References

Follow-up of jupyterlab/lumino#203 and jupyterlab/lumino#206

  • Wait for new release of @lumino/widgets
  • Correct context menu items order to put Paste item after Copy and Cut items
  • Place the New... items on top of the context menu at the bottom (they will be at the same position as before)
  • Remove New Markdown file
  • Add New Notebook items
  • Fix context menu changes due to the new ordering algorithm (in particular ignoring the selector)

The two later tasks will make the context menu more meaningful for JupyterLab as the primary document type is the notebooks and those documents cannot be empty plain text files. The markdown can be added by the New file entry while renaming the extension.

I let the new items at the bottom because looking quickly on various applications, new items are not displayed usually when selecting a folder or a file. They are appearing only when clicking an empty space in the file browser. This goes in the direction of context menu as new items are not directly applying a change on the selected file/folder.

Code changes

The context menu items will only be sorted by rank.

User-facing changes

Items order in the context menu are expected to change.

context menu on file (Before / After)
image

context menu on folder (Before / After)
image

context menu on empty filebrowser space (After)

image

The consequence of this PR is the Paste item that appear on top in the last case (it was on the bottom prior to this PR).

Backwards-incompatible changes

Yes

@jupyterlab-dev-mode
Copy link

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@blink1073
Copy link
Member

Lumino release done

@krassowski
Copy link
Member

Nice! I guess we could either:

  • make a submenu "New", same as in the top-level File menu which would show all three "File", "Notebook", "Markdown", or
  • add isContextMenu argument and condition on it when creating the label so that it shows as "New Notebook":

// Add a command for creating a new notebook.
commands.addCommand(CommandIDs.createNew, {
label: args => {
const kernelName = (args['kernelName'] as string) || '';
if (args['isLauncher'] && args['kernelName'] && services.kernelspecs) {
return (
services.kernelspecs.specs?.kernelspecs[kernelName]?.display_name ??
''
);
}
if (args['isPalette']) {
return trans.__('New Notebook');
}
return trans.__('Notebook');
},

@fcollonval fcollonval force-pushed the ft/contextmenu-rank branch 2 times, most recently from 12fc78b to 2882245 Compare August 11, 2021 10:23
@fcollonval
Copy link
Member Author

Thanks a lot for the review @krassowski I update the label to display New Notebook.

Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

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.

LGTM.

Small note (no action requested): there is a slight inconsistency in how creating the file enters the edit mode on the listing item but creating the notebook does not. It actually makes sense given that for notebook we need to choose kernel first because "Choose kernel" modal blocks user interactions. Maybe we could have New notebookIPython/Xeus/R/Julia and then enter the edit mode immediately, but I am not sure if this would be better.

@fcollonval
Copy link
Member Author

we could have New notebookIPython/Xeus/R/Julia and then enter the edit mode immediately, but I am not sure if this would be better.

This may be tricky as the user may have multiple environments and so multiple choices even for a single language.

Update lumino

Remove eslint warning

Upgrade lumino

Reorder context items

Upload new ui reference screenshots

Fix UI tests

Correct console context menu

Update label for new notebook on context menu
@fcollonval fcollonval merged commit 8ea69f2 into jupyterlab:master Aug 13, 2021
@fcollonval fcollonval deleted the ft/contextmenu-rank branch August 13, 2021 15:29
@github-actions github-actions bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Feb 10, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2022
@fcollonval fcollonval modified the milestones: 4.0, 3.4.0 Apr 13, 2022
@fcollonval
Copy link
Member Author

@meeseeksdev please backport to 3.4.x

@jupyterlab jupyterlab unlocked this conversation May 2, 2022
@fcollonval
Copy link
Member Author

@meeseeksdev please backport to 3.4.x

@lumberbot-app
Copy link

lumberbot-app bot commented May 2, 2022

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout 3.4.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 8ea69f2a273f2ed6d2f9315e803875c91b7fd597
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am "Backport PR #10666: Don't sort context menu items by selector"
  1. Push to a named branch:
git push YOURFORK 3.4.x:auto-backport-of-pr-10666-on-3.4.x
  1. Create a PR against branch 3.4.x, I would have named this PR:

"Backport PR #10666 on branch 3.4.x (Don't sort context menu items by selector)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

fcollonval added a commit to fcollonval/jupyterlab that referenced this pull request May 2, 2022
fcollonval added a commit that referenced this pull request May 3, 2022
* Backport PR #10666: Don't sort context menu items by selector

* Fix UI tests
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.