Skip to content

Fix regression in media layout with multiple images#103

Merged
lfades merged 2 commits intovercel:mainfrom
tywayne:fix/multi-image-layout
Aug 3, 2023
Merged

Fix regression in media layout with multiple images#103
lfades merged 2 commits intovercel:mainfrom
tywayne:fix/multi-image-layout

Conversation

@tywayne
Copy link
Contributor

@tywayne tywayne commented Jul 22, 2023

The changes introduced in #99 inadvertently broke the media layouts when there is more than one image present (reported in #101).

The changes presented here will keep with the spirit of making the base theme match Twitter as closely as possible, while also supporting a grid of multiple images.

@lfades
Copy link
Member

lfades commented Jul 27, 2023

Hi @tywayne 👋

Thank you very much for the PR. I tested it locally and it looks good for 1, 2, and 4 images. The layout for 3 images looks very similar to the one for 2 but it should be more similar to the one for 4.

E.g for https://twitter.com/JosngelGarcaBa3/status/1684679511974293506

image

react-tweet shows

image

4 images looks real nice:

image

@tywayne
Copy link
Contributor Author

tywayne commented Jul 28, 2023

@lfades Ahh yep, I missed that. Latest commit should fix it up.

Copy link
Member

@lfades lfades left a comment

Choose a reason for hiding this comment

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

Amazing stuff. Thank you!

@lfades lfades merged commit 96666ae into vercel:main Aug 3, 2023
@tywayne tywayne deleted the fix/multi-image-layout branch August 4, 2023 03:04
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.

3 participants