-
Notifications
You must be signed in to change notification settings - Fork 509
Chat previews now expanded with aspect ratio #4469
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
Conversation
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. Signed-off-by: Vincent Petry <[email protected]>
|
I'm fine with this, but this is up to @jancborchardt's call :) |
| .loading { | ||
| display: inline-block; | ||
| height: 128px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we once also discussed to use more height, e.g. 300? cc @jancborchardt
But not sure what the endpoint does if the image is actually smalled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can check with it. otherwise there's a "mode=fill" option which I'm not sure what it does, might fit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay I found the ticket with the decision to make it 384px: #3746 (comment)
I'll adjust
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was 128, expanded to 384 so it can fill a bit more width as well and look better. Signed-off-by: Vincent Petry <[email protected]>
Center the spinner as it looks better overall, except for when the picture is narrow in the end. But we can't detect the image size beforehand, so this will do for most use cases. Signed-off-by: Vincent Petry <[email protected]>
|
closing in favor of #4472 which improves the same code further (and I'd like to avoid conflicts) |



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.
Fixes #3746
This was mentionned in #1805 (comment)
Manual Tests