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

New and larger URL preview is a regression imo #16479

Closed
Twi1ightSparkle opened this issue Feb 18, 2021 · 10 comments
Closed

New and larger URL preview is a regression imo #16479

Twi1ightSparkle opened this issue Feb 18, 2021 · 10 comments

Comments

@Twi1ightSparkle
Copy link

Description

matrix-org/matrix-react-sdk#5637 is a regression in my opinion. The larger preview wastes screen real estate because fewer messages is visible at the time
Especially in rss/github rooms. But, also in normal chats. The old small preview was large enough to get a feel for what it was about. If I want to see a larger version I'll click the link

New:
Screen Shot 2021-02-18 at 01 44 48 PM

vs old:
Screen Shot 2021-02-18 at 01 45 08 PM

Steps to reproduce

  • Post a link in any room where inline URL preview is enabled

Logs being sent: no

Version information

develop.element.io
Element version: 29fd47d611c2-react-68933c1a3d42-js-371ca009e9e2

@jryans
Copy link
Collaborator

jryans commented Feb 18, 2021

I think one detail we should consider here is that arbitrary non-image URL previews are quite different from image previews. A URL preview for a non-image should probably use a small left-aligned avatar much more like the previous appearance.

If however we know for certain that a URL is previewing an image, perhaps the new layout with wider image is more apprpropriate.

@callahad
Copy link

Twilight's absolutely right that this takes a problem with link previews and makes it worse (synapse-dev can be borderline unusable even with previews disabled due to bot traffic).

However the smaller image previews are a godsend. Surprisingly significant improvement to my user experience.

@babolivier
Copy link
Contributor

While I agree with the feedback regarding URL previews, it also limits the size of image files sent in the timeline, which I like because it makes the room timeline much more readable. So maybe the right fix would be, rather than reverting the change entirely, to revert it only for URL previews (if the URL isn't the one of an image directly)?

@Twi1ightSparkle
Copy link
Author

Twi1ightSparkle commented Feb 18, 2021

I agree that the smaller image previews are amazing. Maybe that bit can be kept, and the URL inline preview be reverted?

@SimonBrandner
Copy link
Contributor

SimonBrandner commented Feb 18, 2021

I like the bigger URL previews. But in some cases, they're not ideal. I like what @jryans has suggested

I think one detail we should consider here is that arbitrary non-image URL previews are quite different from image previews. A URL preview for a non-image should probably use a small left-aligned avatar much more like the previous appearance.

@ara4n
Copy link
Member

ara4n commented Feb 18, 2021

I am really dubious about this. The bigger URL previews are incredibly dominating and make a bad problem even worse, plus the aesthetics on wordwrapping on the URL preview text is ugly, not aligned with the thumbnail, and consumes even more vertical whitespace while wasting horizontal screen real estate.

Meanwhile it was a very deliberate choice when we designed Riot to go for large immersive image thumbnails, and suddenly unilaterally switching to tiny ones feels opinionated at best.

I suspect the right solution here is going to be to make the thumbnail size adjustable... and not to artifically link URL previews and image thumbnails together, given they are completely different things.

@nadonomy, @jryans, please can we not ship this until we've got aligned.

@jryans
Copy link
Collaborator

jryans commented Feb 18, 2021

@nadonomy, @jryans, please can we not ship this until we've got aligned.

Sure, that's why we're collecting feedback in this issue, which is already marked a release blocker.

@benparsons
Copy link
Member

I'm less concerned about the images (though prefer smaller), but definitely agree that the vertical URL previews make rooms harder to view. Scrolling TWIM right now is quite annoying unfortunately, due to the amount of vertical space.

@jryans
Copy link
Collaborator

jryans commented Feb 22, 2021

I am working with @nadonomy on this, and we'll have more details here before the RC on Wednesday.

@nadonomy
Copy link
Contributor

nadonomy commented Feb 23, 2021

Thanks everyone for the feedback! After chewing the fat on this we've decided to rollback the PR, for several reasons:

  1. To address the feedback from this issue and via other channels
  2. When reviewing the PR, we didn't quite consider how far reaching the changes would be
  3. @SimonBrandner has a draft PR which treats images and URL previews distinctly, which we believe is the right approach
  4. However, to land it, we'll need to consider things like if any character measures or limits are desirable to increase legibility, whether we want to special case any links (e.g. by domain or URL, to do things like display larger thumbnails for videos), the right visual balance for thumbnail sizes, taking into account when we have things like message bubbles, etc

...along with a bunch of other related issues we haven't discovered yet!

So for now, given the timeliness with the RC we're rolling back the changes, closing this issue, and will follow up with the other draft PR to continue to iterate on this.

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

Successfully merging a pull request may close this issue.

8 participants