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

chore(web): improve responsiveness in Album and Shared Album pages on small devices #11055

Merged
merged 3 commits into from
Jul 26, 2024

Conversation

ayykamp
Copy link
Contributor

@ayykamp ayykamp commented Jul 12, 2024

Changes I made:

  • asset-grid: on small screens remove right margin
  • album-title and shared album page: scale font size with viewport
  • album-viewer and album page: less top margin on small screens

must-not-be-named Photos for reference:

Before After
Album Page
Shared Album Page

Comment on lines 427 to 432
<!-- Right margin MUST be equal to the width of immich-scrubbable-scrollbar -->
<section
id="asset-grid"
class="scrollbar-hidden h-full overflow-y-auto outline-none pb-[60px] {isEmpty ? 'm-0' : 'ml-4 tall:ml-0 mr-[60px]'}"
class="scrollbar-hidden h-full overflow-y-auto outline-none pb-[60px] {isEmpty
? 'm-0'
: 'ml-4 tall:ml-0 mr-[20px] md:mr-[60px]'}"
Copy link
Contributor

@michelheusschen michelheusschen Jul 12, 2024

Choose a reason for hiding this comment

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

Just so you know, this component is also used on other pages, such as the main timeline. Reducing the margin causes the asset grid and scrollbar to overlap and when thumbnails have a light color, the scrollbar labels have poor contrast. Maybe we can add a background color to the labels?

Current PR With background
scrollbar-without-bg scrollbar-with-bg

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks good to me. In mnbm Photos they solved it by having the scollbar labels disappear after a short timeout (might try to replicate that in a seperate PR). I've enabled edits by maintainers, I think you can push your changes to my branch.

@bunubbv
Copy link
Contributor

bunubbv commented Jul 13, 2024

I think It might be a good idea to reduce the left and right margins as well.

@ayykamp
Copy link
Contributor Author

ayykamp commented Jul 13, 2024

I think It might be a good idea to reduce the left and right margins as well.

Do you mean like in the reference image? To basically have no margins?

@bunubbv
Copy link
Contributor

bunubbv commented Jul 14, 2024

I think It might be a good idea to reduce the left and right margins as well.

Do you mean like in the reference image? To basically have no margins?

Yes, but instead of having no margins at all like in the reference image, it would be better to reduce the current margins slightly.

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

I'm fine merging this as is with just reverting the margin right change for now and addressing that in a separate pr

@ayykamp
Copy link
Contributor Author

ayykamp commented Jul 18, 2024

I've reverted those changes for now. Might try again in another PR.

@alextran1502 alextran1502 changed the title Improve responsiveness in Album and Shared Album pages on small devices chore(web): improve responsiveness in Album and Shared Album pages on small devices Jul 26, 2024
@alextran1502 alextran1502 enabled auto-merge (squash) July 26, 2024 21:02
@alextran1502 alextran1502 merged commit 147c6e3 into immich-app:main Jul 26, 2024
22 of 23 checks passed
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.

5 participants