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

File editor right-click #8425

Merged

Conversation

karlaspuldaro
Copy link
Contributor

@karlaspuldaro karlaspuldaro commented May 14, 2020

References

#2778 - Improve the right-click menu of the editor

Code changes

New commands added to file editor right-click menu:

  • Undo
  • Redo (new icon added)
  • Cut
  • Copy
  • Paste
  • Select all

User-facing changes

When right clicking on a file editor with a text selection:
image

When there is no text selection, cut and copy buttons are disabled

Backwards-incompatible changes

@jupyterlab-dev-mode
Copy link

Thanks for making a pull request to JupyterLab!

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

@karlaspuldaro karlaspuldaro changed the title File editor right-click [WIP] File editor right-click May 14, 2020
@karlaspuldaro
Copy link
Contributor Author

Note:
Copy command is only enabled if a text selection is detected on the editor. Right-clicking on a different line than the selected line unselects the text. This is a file editor behavior also present with shift+right click menu.

@blink1073 blink1073 marked this pull request as draft May 14, 2020 19:43
@blink1073 blink1073 changed the title [WIP] File editor right-click File editor right-click May 14, 2020
@blink1073
Copy link
Contributor

Thanks @karlaspuldaro! So far this looks good to me overall, can you please take it out of draft when it is ready for a full review?

@blink1073 blink1073 added this to the 2.2 milestone May 14, 2020
@karlaspuldaro
Copy link
Contributor Author

Thank you @blink1073 , will do :)
I thought it would be helpful to start an early PR and get feedback as new commands are added.
I also debated whether each one should have its own issue/PR to limit the feature scope and create individual deliverables.

@blink1073
Copy link
Contributor

I'd say it is fair to add the ones you highlighted here in this PR.

@blink1073 blink1073 modified the milestones: 2.2, 3.0 Jun 11, 2020
@github-actions github-actions bot added tag:Design System CSS If a PR is editing any CSS files please add this tag for design team to review. pkg:ui-components tag:CSS For general CSS related issues and pecadilloes labels Jun 17, 2020
packages/fileeditor-extension/src/commands.ts Outdated Show resolved Hide resolved
packages/fileeditor-extension/src/commands.ts Outdated Show resolved Hide resolved
packages/fileeditor-extension/src/commands.ts Outdated Show resolved Hide resolved
Copy link
Member

@lresende lresende left a comment

Choose a reason for hiding this comment

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

LGTM

@echarles
Copy link
Member

echarles commented Jun 20, 2020

I have tried this PR in --dev mode and it work great! copy/paste, undo/redo do their job. Yeah #8590 would allow to disable undo/redo but this is good for now.

I have also run jupyter lab build but running the build does not show the expected menus. Maybe something on my env... @karlaspuldaro worth to double check and confirm.

Also as follow-up, it would be super to add those copy/paste undo/redo to the cell editor which is already well populated, but miss those basic features.

Screenshot 2020-06-20 at 08 23 13

@karlaspuldaro
Copy link
Contributor Author

Hi @echarles
Thanks for your feedback. I just tried a new build and it works for me, I use a conda env. Did you run lab in dev mode after your build?
I follow the multiple build steps for dev documented here, where jupyter lab build is the very last step.

And I agree it would be helpful to add these features to notebook cells. Is there an open issue for it?

Copy link
Member

@ajbozarth ajbozarth left a comment

Choose a reason for hiding this comment

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

LGTM

@blink1073
Copy link
Contributor

Thanks @karlaspuldaro! @saulshanabrook this is backwards compatible if you want to include it in 2.2.

@saulshanabrook
Copy link
Member

@blink1073 Sounds good!

@saulshanabrook saulshanabrook modified the milestones: 3.0, 2.2 Jun 25, 2020
@saulshanabrook saulshanabrook merged commit 5b9889b into jupyterlab:master Jun 25, 2020
saulshanabrook added a commit that referenced this pull request Jun 25, 2020
@echarles
Copy link
Member

@karlaspuldaro sorry missed your comment. Good to know it works in dev mode. It was for sure something in my env...

@ianhi
Copy link
Contributor

ianhi commented Jun 26, 2020

Hey sorry for the late input on this, there are two things I'd like to bring up:

  1. Awesome!
  2. On Firefox Paste doesn't work on and currently just silently (to the user) fails
    In this gif i'm on firefox 75 on linux and first copy with contextmenu then try pasting with context menu, finally i use crtl-v to paste which does work.
    paste

I believe this is because (https://developer.mozilla.org/en-US/docs/Web/API/Clipboard/readText)

Firefox only supports reading the clipboard in browser extensions, using the "clipboardRead" extension permission.

It's possible that document.exec('paste') would be workaround here?

Also, the extra console.log are not from me (i'm running the current master here) they are from

console.log(current.content.title.label);

@karlaspuldaro
Copy link
Contributor Author

Hi @ianhi
Since this has been merged, would you mind opening an issue for the firefox bug on paste command? I can propose a patch for that.
Thanks for the feedback :)

@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 8, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg:fileeditor pkg:ui-components status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. tag:CSS For general CSS related issues and pecadilloes tag:Design System CSS If a PR is editing any CSS files please add this tag for design team to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants