Skip to content

fix(web): grid on people page - #5640

Merged
jrasm91 merged 8 commits into
mainfrom
fix/grid-people-page
Jan 9, 2024
Merged

jrasm91 merged 8 commits into
mainfrom
fix/grid-people-page

Conversation

@martabal

Copy link
Copy Markdown
Contributor

Changes made in this PR

With this PR, the people grid always takes up the full available width and the size of the thumbnails changes depending on the screen size.

Screenshots

Before After
2023-12-11.21-23-06.mp4
2023-12-11.21-21-13.mp4

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Dec 13, 2023 •

Copy link
Copy Markdown

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4db5a0d
Status: ✅  Deploy successful!
Preview URL: https://d7816658.immich.pages.dev
Branch Preview URL: https://fix-grid-people-page.immich.pages.dev

View logs

@jrasm91 jrasm91 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems OK, but you can actually accomplish all of this with css using flex, not with javascript that has to run in the browser and may cause performance issues on slower devices, like phones.

@martabal

Copy link
Copy Markdown
Contributor Author

This seems OK, but you can actually accomplish all of this with css using flex, not with javascript that has to run in the browser and may cause performance issues on slower devices, like phones.

The thumbnail width is re-calculated only when the width of the div containing all the thumbnails change. So could be an issue only on screen rotation

@martabal
martabal force-pushed the fix/grid-people-page branch from b2ed8a7 to b27b95a Compare January 9, 2024 01:06
@martabal

martabal commented Jan 9, 2024

Copy link
Copy Markdown
Contributor Author

Changed to use grid instead

@martabal
martabal requested a review from jrasm91 January 9, 2024 01:12
@jrasm91

jrasm91 commented Jan 9, 2024

Copy link
Copy Markdown
Member

It's the video updated too or is that the old one?

@jrasm91 jrasm91 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lgtm

@martabal

martabal commented Jan 9, 2024

Copy link
Copy Markdown
Contributor Author

It's the video updated too or is that the old one?

I just changed it

@jrasm91
jrasm91 merged commit 29b204d into main Jan 9, 2024
@jrasm91
jrasm91 deleted the fix/grid-people-page branch January 9, 2024 03:04
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.

4 participants