7878 directly pasting images into edit box and broader drag and drop areas #7883

HankG · 2018-10-08T20:48:48Z

denschub · 2018-10-14T04:04:08Z

@Fensterbank mind having a quick look at this PR if you have some minutes? :)

Fensterbank

Thank you for your pull request. Seems to be a useful change.
I did not yet test the functionality in browsers, I just took a look over the code and found some small syntax and logic things I would like to have improved.

app/assets/javascripts/helpers/post_photo_uploader.es6

…areas

HankG · 2018-10-16T14:13:24Z

@Fensterbank I've addressed and pushed the requested changes

Fensterbank

I tested this PR on my local dev environment and it worked fine. If nobody has additional remarks I think we can merge that.

Finally people can drop images anywhere without having to hit the camera icon and are able to paste images directly from the clipboard. Thanks @HankG, that's a great improvement. 🍪

SuperTux88

Drag and drop works well, but pasting doesn't work as expected:

The filename is still pasted, I think that shouldn't happen, can you remove that behavior?
I had one image, which only worked in chromium, in firefox I got TypeError: blob is null; can't access its "type" property, I don't know if that's a problem with the image or with fineuploader, so I don't know if that's something we can fix, but I still wanted to mention it. The same image works well with drag-and-drop, and other images work also well in firefox with pasting.

Flaburgan · 2018-10-24T07:00:24Z

If it doesn't contain anything private, can you please provide this image so I can try to reproduce?

HankG · 2018-10-24T21:55:40Z

@SuperTux88 You are correct that the filename should no longer be pasted in. Instead it should be pasting in the image for a supported image format or one that it can convert into that format. For example if I had vector art from LibreOffice Draw in the clipboard it would convert it to a PNG and paste that in my testing. All of my Desktop mode testing was in FireFox. All of my Mobile testing was in Chrome. All of that testing was done on Linux Mint MATE. You are having the problem in Firefox? Which OS?

SuperTux88 · 2018-10-25T01:13:00Z

If it doesn't contain anything private, can you please provide this image so I can try to reproduce?

I just created an empty new image in gimp, saved it, and it is broken: here

But it looks like it only happens with big images, 4000x3000 is broken, 3000x2000 works (I didn't do much more testing with different resolutions). It's broken for both: firefox stable and firefox nightly here. But it works in chromium.

Also, while commenting here, I noticed that it's probably nothing we can fix, because I also couldn't copy-paste the broken image here (drag-and-drop worked, but drag-and-drop also works with this PR in diaspora with the broken image). And I also just tested it in rocketchat (the only other thing that came to my mind where I ever used copy-paste images), and it's broken there too. So it looks like it's something broken with the browser? I usually only use copy-paste from firefox-screenshots, which are usually much smaller, because I only screenshot parts of a page, so I never noticed that before.

You are correct that the filename should no longer be pasted in.

I also just noticed that this happens here on GitHub too, so it's probably also something we can't fix? I usually only use copy paste directly from firefox-screenshots, where the image doesn't have a filename/path yet, so there is nothing to paste, so I never noticed that. When opening a file, copy the content, it has a path which then is pasted together with the image.

So I think everything is fine, and I just tested stuff I usually don't do and didn't know the problems/behavior exist everywhere else too.

SuperTux88

So only a little thing :) otherwise I think we can merge this, because as said in the comment above, I think the other stuff is nothing we can fix.

SuperTux88 · 2018-10-25T01:15:32Z

app/assets/javascripts/helpers/post_photo_uploader.es6

@@ -53,6 +54,10 @@ Diaspora.PostPhotoUploader = class {
          /* eslint-enable camelcase */
        }
      },
+      paste: {
+        targetElement: document.getElementById("status_message_text"),
+        promptForName: true


Just a little thing: I looked what this option does, and it should open a prompt for asking for a filename. It neither opens a prompt nor do I think we need a prompt. The filename doesn't matter, so I suggest to remove this option.

IIRC with that field set to false the request didn't percolate up correctly to initiate our handlers it just pasted into the text field like before. I can double check that that was the case. If it is there is nothing I can do about it even though it is confusing.

I confirmed what I stated above.

SuperTux88 · 2018-10-25T01:25:54Z

I just created an empty new image in gimp, saved it, and it is broken

Oh, and I just tested that image on another computer and there it works, even with firefox, but here I can download it and I can't upload it to diaspora. The only difference is that one computer has i3wm and the other has awesomewm. I have no idea how this image-copy-paste magic works and what needs to work together for it to work ;) But I think we can ignore that problem for this PR, because it's obviously a problem on my side ;)

HankG · 2018-10-25T03:42:28Z

I just created an empty new image in gimp, saved it, and it is broken

Oh, and I just tested that image on another computer and there it works, even with firefox, but here I can download it and I can't upload it to diaspora. The only difference is that one computer has i3wm and the other has awesomewm. I have no idea how this image-copy-paste magic works and what needs to work together for it to work ;) But I think we can ignore that problem for this PR, because it's obviously a problem on my side ;)

I think I may feel better if it's tested on a Windows and a Mac under Chrome, Firefox, and Safari if we are having some differences between window managers. I won't be able to do that until early next week though since I'm on travel right now. If someone else knocks that out before then that's fine too of course :)

Flaburgan · 2018-10-25T07:26:37Z

I have access to any browser / os configuration thanks to browserstack. I'll test this PR.

Flaburgan

I'm so sorry I took me that long to test the different browsers versions on that code. I forgot I had to do that. Fortunately as this is targeted for next major release this oversight didn't have any consequences but still sorry about it.

The good news is, it works very well! I tested copy paste and drag and drop on Windows 10 with Edge 18, Chrome 74 and Firefox 66 and on Mac OS with Safari 12, Firefox 66 and Chrome 74. The only thing I noticed is that drag and drop wasn't working between two browser tabs in Chrome and Firefox on both Windows and Mac. It displayed the message "No file to upload" but it worked from the windows explorer so I guess we're good.

Two last minor things, in the console, each time I drop a file, I have the "Grabbed 1 dropped files." message displayed, can this be removed? Also, when dropping an image to the reduced publisher, it doesn't expand. Can this be changed?

michapixel · 2019-05-28T13:51:17Z

Can this be changed?
If - i haven't looked it up - your really using fineuploader (like they seem to debug this line, as google told me)
Hereyego:

https://docs.fineuploader.com/quickstart/02-setting_options.html

{ debug:false }

tclaus · 2020-11-10T21:14:00Z

Works for me (on a user perspective) well, too. I rebased to latest diaspora/develop and tried it out.
Drag&Drop of images is a great feature I'd like to have it in the 8'.

There are still some points open for review - can someone clarify the need to fix this?

Grabbed 1 dropped files." in console after dropping a file
Drag and Drop between Browsers
Publisher doesn't expand if dragging to a reduced publisher

For the first point I found no direct way to get rid of the message. The hint 'debug:false' seems to have no effect
Second: Drag and drop between browser is nothing I would expect as a user to work.

Third: (automatic open of a publisher) is this really a blocker?
@Flaburgan can you comment this?

tclaus · 2021-10-21T06:14:41Z

@SuperTux88 Can you have a look here and possible approve this? For me there are no more tasks left.

SuperTux88 · 2021-10-22T23:06:30Z

I still have the same issues as before:

Copying a image-file and then pasting into the publisher just pastes the path
Opening the image in an image viewer and copying the content and then pasting it, uploads it, but also still pasts the path in addition (this time prefixed with file://)

But I don't know, maybe that's just a firefox/linux issue and I'm the only one having them? If it's working for everybody else I'm fine with it.

Drag-and-drop works, and also pasting a screenshot that was directly copied and not stored to a file first (which is my most used workflow) both works as expected.

And then there is still the issue with not opening the publisher when dragging a file into it when it's closed. The upload still works, but it stays closed, so you don't see that it worked, which is definitely bad UX. But since I think overall it's still an improvement to the old state, so I guess we can fix the publisher opening in a follow up PR?

tclaus · 2021-10-24T08:40:22Z

Thanks for watching again. With this amount of open and follow-up PRs I am beginning to loose the overview.
With #8237 a follow-Up of exists. The problem is the same, but I invetigetad the issue you mentioned and found out. that I did not found the source code which caused the problem.
By the ways - lets merge this (its not worse than before) but it adds a new and often requested feature.

So maybe this PR might be closed as all is shifted to #8237 ?

HankG mentioned this pull request Oct 8, 2018

Directly pasting image into a post when composing #7878

Closed

Fensterbank suggested changes Oct 16, 2018

View reviewed changes

SuperTux88 added this to the 0.8.0.0 milestone Oct 16, 2018

7878 directly pasting images into edit box and broader drag and drop …

918c45b

…areas

HankG force-pushed the 7878-direct-image-pasting branch from 66384b0 to 918c45b Compare October 16, 2018 14:13

Fensterbank approved these changes Oct 18, 2018

View reviewed changes

SuperTux88 changed the title ~~7878 directly pasting images into edit box and broader drag and drop …~~ 7878 directly pasting images into edit box and broader drag and drop areas Oct 23, 2018

SuperTux88 requested changes Oct 23, 2018

View reviewed changes

SuperTux88 requested changes Oct 25, 2018

View reviewed changes

denschub assigned Flaburgan Apr 28, 2019

Flaburgan suggested changes May 4, 2019

View reviewed changes

Flaburgan assigned HankG and unassigned Flaburgan May 4, 2019

jhass requested a review from SuperTux88 November 11, 2020 08:28

tclaus mentioned this pull request Apr 15, 2021

7878 direct image pasting #8237

Merged

SuperTux88 approved these changes Oct 22, 2021

View reviewed changes

SuperTux88 closed this in 36e6b31 Oct 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

7878 directly pasting images into edit box and broader drag and drop areas #7883

7878 directly pasting images into edit box and broader drag and drop areas #7883

HankG commented Oct 8, 2018 •

edited by SuperTux88

denschub commented Oct 14, 2018

Fensterbank left a comment

HankG commented Oct 16, 2018 •

edited

Fensterbank left a comment

SuperTux88 left a comment

Flaburgan commented Oct 24, 2018

HankG commented Oct 24, 2018 •

edited

SuperTux88 commented Oct 25, 2018

SuperTux88 left a comment

SuperTux88 Oct 25, 2018

HankG Oct 25, 2018

HankG Dec 29, 2018

SuperTux88 commented Oct 25, 2018

HankG commented Oct 25, 2018 •

edited

Flaburgan commented Oct 25, 2018

Flaburgan left a comment

michapixel commented May 28, 2019

tclaus commented Nov 10, 2020 •

edited

tclaus commented Oct 21, 2021

SuperTux88 commented Oct 22, 2021

tclaus commented Oct 24, 2021

7878 directly pasting images into edit box and broader drag and drop areas #7883

7878 directly pasting images into edit box and broader drag and drop areas #7883

Conversation

HankG commented Oct 8, 2018 • edited by SuperTux88

denschub commented Oct 14, 2018

Fensterbank left a comment

Choose a reason for hiding this comment

HankG commented Oct 16, 2018 • edited

Fensterbank left a comment

Choose a reason for hiding this comment

SuperTux88 left a comment

Choose a reason for hiding this comment

Flaburgan commented Oct 24, 2018

HankG commented Oct 24, 2018 • edited

SuperTux88 commented Oct 25, 2018

SuperTux88 left a comment

Choose a reason for hiding this comment

SuperTux88 Oct 25, 2018

Choose a reason for hiding this comment

HankG Oct 25, 2018

Choose a reason for hiding this comment

HankG Dec 29, 2018

Choose a reason for hiding this comment

SuperTux88 commented Oct 25, 2018

HankG commented Oct 25, 2018 • edited

Flaburgan commented Oct 25, 2018

Flaburgan left a comment

Choose a reason for hiding this comment

michapixel commented May 28, 2019

tclaus commented Nov 10, 2020 • edited

tclaus commented Oct 21, 2021

SuperTux88 commented Oct 22, 2021

tclaus commented Oct 24, 2021

HankG commented Oct 8, 2018 •

edited by SuperTux88

HankG commented Oct 16, 2018 •

edited

HankG commented Oct 24, 2018 •

edited

HankG commented Oct 25, 2018 •

edited

tclaus commented Nov 10, 2020 •

edited