Add support for dragging attachments from file browser and pasting #5913

Madhu94 · 2019-01-27T09:45:15Z

Will fix #5744

Allow dragging files from filebrowser to notebook
Allow pasting images into notebook
Handle native drop of files
Modify markdown cell's source to append the attachment (doing this may need moving of drag and paste event listeners into markdown cell widget, instead of notebook widget?)
If a cell attachment is not referenced in any of the cell's outputs, remove that attachment on save

Madhu94 · 2019-03-02T15:52:58Z

Hi all, this is mostly complete. It would be really helpful if this could be reviewed. Thank you!

vidartf · 2019-03-04T14:29:13Z

Is there a way to get the binder-bot to make a link here (or does anybody know how to do it manually)?

vidartf · 2019-03-04T15:48:38Z

Ah, yes: https://mybinder.org/v2/gh/Madhu94/jupyterlab/add-attachments?urlpath=lab-dev

vidartf

Looking at the binder, I see that there is an issue when dragging an image from the file browser: The default behavior of the image being dragged as a tab for viewing is not cancelled when dragging over a cell. It correctly drops the file as an attachment, but the blue box is then stuck on screen until you start a new widget drag operation.

See also specific code comment about trying to coordinate with https://github.com/agoose77/jupyterlab-attachments .

packages/cells/src/widget.ts

jasongrout · 2019-03-04T16:17:08Z

The default behavior of the image being dragged as a tab for viewing is not cancelled when dragging over a cell. It correctly drops the file as an attachment, but the blue box is then stuck on screen until you start a new widget drag operation.

IIRC, this is because of this phosphor bug: phosphorjs/phosphor#300

Madhu94 · 2019-03-04T16:34:01Z

IIRC, this is because of this phosphor bug: phosphorjs/phosphor#300

@jasongrout Thank you so much, I couldn't figure this out ! This was the reason I added [WIP] to the PR.

It seems like this isn't a trivial fix though? Should we use a UI similar to @agoose77's extension, until we fix the phosphor issue? This extension uses the context menu for lab's filebrowser, not the phosphor drag and drop.

jasongrout · 2019-03-04T16:47:54Z

It seems like this isn't a trivial fix though?

Well, I think the fix is as easy as merging that PR and releasing a phosphor patch update :), but apparently others may disagree? I agree it's not easy to fix outside of phosphor, though perhaps @sccolbert's idea of using multiple mimetypes mentioned in the issue may also be a way to do it. I don't think I tried that, though apparently I thought through it a bit in our conversation several years ago.

Madhu94 · 2019-03-09T09:10:38Z

@jasongrout Multiple mime types are being used here, so I am not sure if I've understood right. application/vnd.phosphor.widget-factory is the mime type that the dockpanel uses, do you mean maybe we can choose to remove it if the drop is handled by a markdown cell widget ? A markdown cell widget would use a different custom mime type (like application/vnd.jupyter.attachments in this PR)

vidartf

Mostly looking OK, but minor fixes and tweaks. Also, there is still the outstanding issue in the OP about removing attachments when the references are dropped (probably on MD render and/or save/serialization?).

packages/cells/src/widget.ts

vidartf · 2019-04-04T21:21:41Z

packages/cells/src/widget.ts

+      if (protocol !== 'data:') {
+        return;
+      }
+      const content = href.split(':')[1];


I think you can use pathname from URLExt.parse instead :)

This had an issue with firefox where pathname came out to be '', I'm not sure why

packages/cells/src/widget.ts

vidartf · 2019-04-04T22:19:44Z

Also: When testing on Binder with Chrome, I couldn't get pasting of files from the system clipboard to work (pasting was greyed out in the system context menu, and Ctrl+V didn't do anything). Is this expected?

agoose77 · 2019-04-05T07:47:24Z

@Madhu94 I'm wondering, does the 'drop non-referenced attachments' feature exist in classic notebook? If not, could this lead to UX confusion for users?

vidartf · 2019-04-29T10:46:01Z

@agoose77 Yes, it's been in from the start. See here: https://github.com/jupyter/notebook/blob/6a96754208aa054a91781eabff06cf89e1b1e37c/notebook/static/notebook/js/textcell.js#L220-L274

jasongrout · 2019-06-14T00:33:18Z

@fperez mentions that this might be considered a 1.0 issue, in that it is a missing feature from the classic notebook.

blink1073 · 2019-06-14T03:36:42Z

Counterpoint: this feature didn't exist in notebook when we made a 1.0 milestone.

jasongrout · 2019-06-14T04:29:38Z

@Madhu94 - how close do you think this is to being finished?

Madhu94 · 2019-06-19T19:50:37Z

[Apologies for my long absence from Github]

I took care of the first set of review comments, pending work is removing unreferenced attachments (on it now) and the phosphor issue mentioned in this comment (IMO, more of a usability issue than unreferenced attachments)

Please add any other review comments, if needed

Madhu94 · 2019-06-19T19:54:59Z

Also: When testing on Binder with Chrome, I couldn't get pasting of files from the system clipboard to work (pasting was greyed out in the system context menu, and Ctrl+V didn't do anything). Is this expected?

Strange. Can you tell me how you tested ? I opened random images from Google images, clicked 'Copy Image' from the right-click context menu and then did Ctrl+V after focusing on the markdown cell in the notebook.

Madhu94 · 2019-09-04T18:47:23Z

Potentially, if we are being strict about things,

@vidartf Just for my curiosity, could you please explain what issues you foresee? In case I have to upgrade stuff again, in the future

files from filebrowser, native drag/drop or by pasting

@phosphor/widgets@1.9.0 has a bug fix - dockpanel overlay did not hide itself even if markdown cell handled the p-drop event.

Change from a custom attachments mimetype to mimetype of the contents model with the suffix ";closure=true"

Adds a rich contents mime type to allow extensions to more directly use the already fetched content models of the filebrowser. This is then used in the attachment cell widgets to synchronously insert the name (which will initially resolve to a broken image), and then start the async fetch. Also reefactors `updateCellSourceWithAttachment`.

Madhu94 · 2019-09-05T11:29:29Z

Whoo ! Finally ! :) Thanks for your help @vidartf .

vidartf · 2019-09-05T11:33:04Z

Thanks for you efforts and your patience @Madhu94 :)

ian-r-rose · 2019-09-05T22:05:30Z

Thanks for the incredible work and patience @Madhu94!

jasongrout · 2019-09-28T06:26:41Z

Did this also sove #7085?

Madhu94 mentioned this pull request Jan 27, 2019

[Feature request] drag-and-drop files to markdown cells for attachments #5744

Closed

Madhu94 force-pushed the add-attachments branch from b2ea39c to c362dfd Compare March 2, 2019 14:58

vidartf suggested changes Mar 4, 2019

View reviewed changes

packages/cells/src/widget.ts Outdated Show resolved Hide resolved

afshin requested review from tgeorgeux and vidartf March 27, 2019 16:55

afshin added this to the 1.0 milestone Mar 27, 2019

vidartf suggested changes Apr 4, 2019

View reviewed changes

blink1073 assigned vidartf May 23, 2019

jasongrout added status:Work in Progress and removed status:Work in Progress labels May 29, 2019

Madhu94 force-pushed the add-attachments branch 2 times, most recently from ad5faf6 to d8d4525 Compare June 1, 2019 19:38

blink1073 modified the milestones: 1.0, 1.1 Jun 9, 2019

Madhu94 force-pushed the add-attachments branch from d8d4525 to 9ecee8d Compare June 19, 2019 17:55

Madhu94 and others added 14 commits September 4, 2019 21:12

Allow cell attachments to be added to the cell by dragging

dcc1578

files from filebrowser, native drag/drop or by pasting

Use regex for datauri parsing

9ed44e7

Add support for removing attachments

ae78e21

Stop dragover event from propagating

97bedb6

Upgrade to latest phosphor packages

9554a7d

@phosphor/widgets@1.9.0 has a bug fix - dockpanel overlay did not hide itself even if markdown cell handled the p-drop event.

Remove check based on model.name

8e8362a

Lint

ceaa00c

Thunk typing

d4813c5

Lints

e18518e

Minor cleanup

fa588ae

Change mimetype set on drag event.

3496cce

Change from a custom attachments mimetype to mimetype of the contents model with the suffix ";closure=true"

Add TODO

1f9bfcf

Remove rendermine dependency in filebrowser

9fe9968

vidartf force-pushed the add-attachments branch from fd1d764 to cea5098 Compare September 4, 2019 20:25

integrity

b874b3a

vidartf force-pushed the add-attachments branch from cea5098 to b874b3a Compare September 4, 2019 20:28

vidartf approved these changes Sep 4, 2019

View reviewed changes

vidartf merged commit 8cd72f7 into jupyterlab:master Sep 5, 2019

jasongrout removed the status:Work in Progress label Sep 10, 2019

jasongrout modified the milestones: 2.0, 1.2 Sep 10, 2019

lresende mentioned this pull request Oct 26, 2019

Add capability to drag file from filebrowser into pipeline editor elyra-ai/elyra#27

Closed

lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Oct 28, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for dragging attachments from file browser and pasting #5913

Add support for dragging attachments from file browser and pasting #5913

Madhu94 commented Jan 27, 2019 •

edited

Madhu94 commented Mar 2, 2019

vidartf commented Mar 4, 2019

vidartf commented Mar 4, 2019

vidartf left a comment

jasongrout commented Mar 4, 2019

Madhu94 commented Mar 4, 2019

jasongrout commented Mar 4, 2019

Madhu94 commented Mar 9, 2019 •

edited

vidartf left a comment

vidartf Apr 4, 2019

Madhu94 Jun 19, 2019

vidartf commented Apr 4, 2019

agoose77 commented Apr 5, 2019 •

edited

vidartf commented Apr 29, 2019

jasongrout commented Jun 14, 2019

blink1073 commented Jun 14, 2019

jasongrout commented Jun 14, 2019

Madhu94 commented Jun 19, 2019 •

edited

Madhu94 commented Jun 19, 2019

Madhu94 commented Sep 4, 2019 •

edited

Madhu94 commented Sep 5, 2019

vidartf commented Sep 5, 2019

ian-r-rose commented Sep 5, 2019

jasongrout commented Sep 28, 2019

Add support for dragging attachments from file browser and pasting #5913

Add support for dragging attachments from file browser and pasting #5913

Conversation

Madhu94 commented Jan 27, 2019 • edited

Madhu94 commented Mar 2, 2019

vidartf commented Mar 4, 2019

vidartf commented Mar 4, 2019

vidartf left a comment

Choose a reason for hiding this comment

jasongrout commented Mar 4, 2019

Madhu94 commented Mar 4, 2019

jasongrout commented Mar 4, 2019

Madhu94 commented Mar 9, 2019 • edited

vidartf left a comment

Choose a reason for hiding this comment

vidartf Apr 4, 2019

Choose a reason for hiding this comment

Madhu94 Jun 19, 2019

Choose a reason for hiding this comment

vidartf commented Apr 4, 2019

agoose77 commented Apr 5, 2019 • edited

vidartf commented Apr 29, 2019

jasongrout commented Jun 14, 2019

blink1073 commented Jun 14, 2019

jasongrout commented Jun 14, 2019

Madhu94 commented Jun 19, 2019 • edited

Madhu94 commented Jun 19, 2019

Madhu94 commented Sep 4, 2019 • edited

Madhu94 commented Sep 5, 2019

vidartf commented Sep 5, 2019

ian-r-rose commented Sep 5, 2019

jasongrout commented Sep 28, 2019

Madhu94 commented Jan 27, 2019 •

edited

Madhu94 commented Mar 9, 2019 •

edited

agoose77 commented Apr 5, 2019 •

edited

Madhu94 commented Jun 19, 2019 •

edited

Madhu94 commented Sep 4, 2019 •

edited