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

Improve previews for guests + GIF support #4472

Merged
merged 5 commits into from
Oct 29, 2020

Conversation

PVince81
Copy link
Member

@PVince81 PVince81 commented Oct 26, 2020

As this touched the same code, I've gathered them into a single PR.

Enhancements

Improve preview cropping

Chat previews now expanded with aspect ratio

Instead of cropping the previews in the conversation to a square, they are now using a limited height but their width is computed based on aspect ratio. This is done by passing different arguments to the previews endpoint so it delivers a preview with the matching size.

Also expanded image preview height to 384 as it looks better.

Fixes #3746

Render GIFs directly

Use direct URL to render GIFs (animated!) when they have a small size.

The maximum size is configurable with occ config:app:set spreed max-gif-size --value=X and exposed as the capability spreed.config.previews.max-gif-size

Fixes #1805

Add support for guest previews

Guest users can now directly see images in the chat, and small GIFs are animated as well!

Fixes #4471

@PVince81
Copy link
Member Author

PVince81 commented Oct 26, 2020

  • BUG: previews in upload dialog are too big now

@PVince81 PVince81 force-pushed the enh/noid/improve-previews-guests-and-gifs branch from 98ea8bf to 6cadf89 Compare October 26, 2020 22:04
@PVince81
Copy link
Member Author

  • BUG: need a max-width for when the preview is too wide, otherwise it pushes the increases chat width => I've added a max-width: 100% to the preview (squashed)

@PVince81 PVince81 force-pushed the enh/noid/improve-previews-guests-and-gifs branch from 6cadf89 to fc78669 Compare October 26, 2020 22:17
@PVince81
Copy link
Member Author

  • squash after review

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Works fine!
Maybe we should have width/height constrains, it can lead to really big images
dev skjnldsv com_apps_spreed_not-found

@PVince81
Copy link
Member Author

@skjnldsv the height will always be the same. For the width I added max-width: 100% so it doesn't become wider than the chat window. Not sure if we need more.

Thanks for the approval, I'll squash.

@PVince81 PVince81 force-pushed the enh/noid/improve-previews-guests-and-gifs branch from 6156257 to 2d77bfd Compare October 27, 2020 09:02
@jancborchardt
Copy link
Member

Very nice @PVince81! :) Tiny enhancements which would make it look even better (but you decide whether to do it here or in a separate pull request):

  • Give images a border-radius: var(--border-radius) so it looks more integrated
  • Cut the image name from below since it looks technical, rarely has value and other chat solutions don’t show it either. Can still be shown as "title" attribute if we think it’s necessary.

@PVince81
Copy link
Member Author

PVince81 commented Oct 27, 2020

Cut the image name from below since it looks technical

@jancborchardt also see related discussion about renaming files before upload. If we remove the display name then renaming the files is maybe not needed any more: #3543

@PVince81
Copy link
Member Author

PVince81 commented Oct 28, 2020

  • test image layout when conversation happens in sidebar

PVince81 and others added 4 commits October 28, 2020 16:59
Instead of cropping the previews in the conversation to a square, they
are now using a limited height but their width is computed based on
aspect ratio. This is done by passing different arguments to the
previews endpoint so it delivers a preview with the matching size.

Adjusted styles around the preview to make it look better.

Expanded image preview height to 384 as it looks better.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Use direct URL for small GIFS.
Added preview support for guest mode.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Added app setting and capability "max-gif-size".
Added new import for @nextcloud/capabilities.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Co-authored-by: John Molakvoæ <skjnldsv@users.noreply.github.com>
@PVince81
Copy link
Member Author

PVince81 commented Oct 28, 2020

Note: to test sidebar mode need to merge with #4291 and upload the files through the regular talk view.

  • BUG?: small pictures get stretched (ex: 50x50 pixels)
    • it seems this was already the case before but was less noticable
  • BUG?: image is cropped in sidebar mode instead of having its size reduced:
    • reduce size in sidebar mode
    • maybe we can use a smaller vertical height there instead

Makes it possible to resize both previews and GIFs in a responsive
manner, keeping the ratio and everything visible.

Added border radius to make it look even nicer.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@PVince81 PVince81 force-pushed the enh/noid/improve-previews-guests-and-gifs branch from 2d77bfd to 7c26560 Compare October 28, 2020 16:30
@PVince81
Copy link
Member Author

The issue was that sometimes the height is anyway the one we got from the preview, but sometimes when we render the real image like a GIF, we need max-height.

With the latest commit the image resizing and keeping the ratio is solved, both in the sidebar and also regular chat view.
Smaller pictures (like 50x50) are rendered small, which also means people could in theory upload small pictures of custom emojis if they wanted to without them being stretched...

@skjnldsv mind having another glance with your GIF collection and play with the resize ?

@@ -4919,6 +4919,7 @@
"resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-4.3.0.tgz",
"integrity": "sha512-zbB9rCJAT1rbjiVDb2hqKFHNYLxgtk8NURxZ3IZwD3F6NtxbXZQCnnSi1Lkx+IDohdPlFp222wVALIheZJQSEg==",
"dev": true,
"optional": true,
Copy link
Member

Choose a reason for hiding this comment

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

what was it with the optional again to not get it in and out again all the time?

Copy link
Member Author

Choose a reason for hiding this comment

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

no idea, this got set automatically when installing the new package

@skjnldsv
Copy link
Member

skjnldsv commented Oct 29, 2020

Peek 29-10-2020 08-55

A weird display bug for the second (75Mo gif)

Otherwise works fine. Maybe we should add an indicator to why it wont auto play if it's over 3Mo

@PVince81 PVince81 merged commit 63ab1d6 into master Oct 29, 2020
@PVince81 PVince81 deleted the enh/noid/improve-previews-guests-and-gifs branch October 29, 2020 08:07
@nickvergessen
Copy link
Member

A weird display bug for the second (75Mo gif)

I see this too sometimes on our instance, I guess this happens when generating the preview takes too long or errors on first attempt or something. But it's a server issue

@skjnldsv
Copy link
Member

A weird display bug for the second (75Mo gif)

I see this too sometimes on our instance, I guess this happens when generating the preview takes too long or errors on first attempt or something. But it's a server issue

But the mime icon is really small now, no?

@nickvergessen
Copy link
Member

Well the icon you see there is the placeholder icon in case of an error I think. because the mimetype icon of svg is the image icon.

@PVince81
Copy link
Member Author

PVince81 commented Nov 5, 2020

here's a fix for the icon height: #4537

@PVince81
Copy link
Member Author

PVince81 commented Nov 5, 2020

and here the play button to make it clearer that it can be played: #4538

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

Successfully merging this pull request may close these issues.

Previews do not render for guests Better preview of the shared files Play gifs
4 participants