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

Image preview when pasting or adding images via selector #398

Merged
merged 21 commits into from Nov 29, 2016

Conversation

averissimo
Copy link
Member

@averissimo averissimo commented Sep 27, 2016

This improves current functionality and is only implemented when pasting images.

It allows to paste image and send both the image and current text with "enter" or clicking the image.

output

Open questions / TODO

  • Done see comment below: How to do this with multiple images? Once we figure this one out, I guess it becomes mergeable
    • The image selector allows to upload multiple images and that is why It is not implemented yet
  • How to allow pasting even when input is not selected
    • Should we have a system wide event handler for pasting that always paste to conversation (if conversation is the active interface)
  • I don't like the fact that this implementation requires a div over 'emoji container', but otherwise it would have a conflict with it

Testing

I've done some testing, but need further :)

edit1: adds figure
edit2: mark first point of TODO as done and changed title

- allows to add image from file selector, in ubuntu it only allows
   for one file at a time
- cleanup some leftover code I forgot to remove once moved to
   input.coffee
@averissimo
Copy link
Member Author

averissimo commented Sep 27, 2016

While using YakYak I was sending a friend an image from disk and realized that it does not allow to select more than a single file :)

A fast coding sprint later it is implemented and with a fallback if by any chance multiple files are selected it default to previous behavior that would send them all

outfznvnf6jki

edit: I used the same workflow as with clipboard by embed the image in the application, instead of referencing the path.

This was a personal choice, as I prefer to visually see what I'm sending and polling the file for changes would be complex (I think). Of course I can change to use path if you guys feel that it is better.

@averissimo averissimo changed the title [DO NOT MERGE] Image preview when pasting Image preview when pasting or adding images via selector Sep 27, 2016
@averissimo
Copy link
Member Author

Also added drag and drop visual support (actual implementation was already done)

outumy0pxgsvg

@averissimo
Copy link
Member Author

noticed it was lacking a pointer when hovering the send image button.

  • adds pointer
  • adds animation with color change of button (and back to normal)
  • ditches gray square to have a cleaner look
  • green button is now a gray with transparency

outmsgzqi6lrq

@davibe
Copy link
Contributor

davibe commented Sep 29, 2016

@averissimo your work is great. So fare i have only looked at the super-amazing screencasts. This is what comes to my mind

  • maybe "Drop here" overlay image should be limited to the conversation instead of covering the entire application
  • maybe the play (>) icon is not the best for "upload). I think something shaped like this would look better (besides color etc) http://www.freeiconspng.com/uploads/upload-icon-11.jpg

@averissimo
Copy link
Member Author

averissimo commented Sep 29, 2016

These are the icons available in the material icons font. The first one is the official hangouts icon for this purpose

Not really sure which is better. Some more options at the link below: (all CreativeCommons)

https://thenounproject.com/search/?q=send

Any suggestions on color? play with it :)

selection_00066
selection_00067
selection_00068
selection_00065

@averissimo
Copy link
Member Author

moved the overlay to the conversation. drag target is also just the conversation, i.e. cannot drop in contacts :)

@averissimo averissimo mentioned this pull request Oct 7, 2016
@averissimo
Copy link
Member Author

averissimo commented Oct 10, 2016

Does not work with GIFs (as the current implemented workflow always converts images to PNG)

I guess it can't be merged until this is fixed.

Actually nativeImage does not support GIF at the moment

edit: can now upload gif using drag/drop or image selector (no paste support yet)

@averissimo
Copy link
Member Author

averissimo commented Oct 11, 2016

Electron's native-image does not support GIF yet electron/electron#5078

It is possible to implement it by either:

  • changing the clipboard module to a nodejs
  • see if it comes from html and download image directly

Or we could just keep this as known bug and wait until electron implements it.

ps. right now it creates a static PNG from the gif

@Bacto
Copy link

Bacto commented Dec 16, 2016

Hi @averissimo,

Do you know a nodejs module that can copy a GIF to clipboard?
I can't find one unfortunately :(

@averissimo
Copy link
Member Author

averissimo commented Dec 16, 2016

@Bacto I think electron webContents has a function that copies an image at position X,Y.. I haven't tried it it yet though so I don't know how well it works with static images or gifs

image

@averissimo
Copy link
Member Author

averissimo commented Dec 16, 2016

I have no idea about a general nodejs module

@Bacto
Copy link

Bacto commented Dec 16, 2016

Unfortunately I want to copy an animated GIF so it will not do the job.
But thank you for your reply :)

@averissimo
Copy link
Member Author

so that functions copies the image as a NativeImage?

@averissimo
Copy link
Member Author

averissimo commented Dec 16, 2016

yeah I just tested it, it copies the frame it is on, never the entire gif :\

anyway, thanks for sparkling my interest in this, we'll probably add copy image feature soon (even if it is only static ones)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants