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

feat(web): Context menu scrolls on small devices #15367

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

matitalatina
Copy link
Contributor

@matitalatina matitalatina commented Jan 15, 2025

Description

Context menu can scroll on small devices

Screenshots

This is before the change

before

This is after the change

after

How I tested

Tested on:

  • Android Samsung Galaxy S10e device
  • iPhone 12 Pro
  • Macbook Pro (to check nothing breaks on bigger screen)

class:max-h-0={!isVisible}
class="flex flex-col transition-all duration-[250ms] ease-in-out outline-none"
class="{isVisible
? 'max-h-screen max-h-svh'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put both 100vh and 100svh for retro compatibility.
I preferred svh instead of dvh because it should be more stable reading the CSS standard.

The sizes of the dynamic viewport-percentage units are not stable even while the viewport itself is unchanged. Using these units can cause content to resize e.g. while the user scrolls the page. Depending on usage, this can be disturbing to the user and/or costly in terms of performance.

@matitalatina matitalatina force-pushed the scroll-on-small-devices branch from 182c9d3 to faf3f49 Compare January 15, 2025 20:17
@alextran1502 alextran1502 merged commit 81568db into immich-app:main Jan 15, 2025
33 checks passed
@matitalatina matitalatina deleted the scroll-on-small-devices branch January 15, 2025 20:51
vladd11 pushed a commit to vladd11/immich that referenced this pull request Jan 25, 2025
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