Skip to content

Conversation

@uhthomas
Copy link
Collaborator

@uhthomas uhthomas commented Feb 6, 2026

Description

The existing implementation for showing asset details uses a bottom sheet, and is not in sync with the preview or scroll intent. Other apps use inline details, which is much cleaner and feels better to use.

How Has This Been Tested?

Tested in an emulator.

Screenshots (if appropriate)

demo2.mp4

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if applicable
  • I have no unrelated changes in the PR.
  • I have confirmed that any new dependencies are strictly necessary.
  • I have written tests for new code (if applicable)
  • I have followed naming conventions/patterns in the surrounding code
  • All code in src/services/ uses repositories implementations for database calls, filesystem operations, etc.
  • All code in src/repositories/ is pretty basic/simple and does not have any immich specific logic (that belongs in src/services/)

Please describe to which degree, if any, an LLM was used in creating this pull request.

I did use Claude Code to bounce ideas off of, but most of the code is written by me, and any code written with the help of Claude has been reviewed.

@uhthomas uhthomas requested review from alextran1502 and shenlong-tanwen and removed request for shenlong-tanwen February 6, 2026 01:06
@uhthomas uhthomas force-pushed the uhthomas/mobile-feat-inline-asset-details branch 13 times, most recently from c357f0b to 2f68c95 Compare February 6, 2026 04:10
@uhthomas uhthomas requested a review from mertalev February 6, 2026 11:47
@uhthomas uhthomas force-pushed the uhthomas/mobile-feat-inline-asset-details branch 3 times, most recently from 34db059 to c230c9c Compare February 8, 2026 19:09
@alextran1502
Copy link
Member

Hello, I just ran a quick test, and there are two points that need to be changed

  1. When swiping up, the top system app bar should become transparent to give a better view area to the current photo; it is currently painted as either white or black, depending on the theme mode.
  2. When the bottom sheet is present, and you swipe on the photo to navigate next/prev, it will dismiss the bottom sheet. The current behavior on main is that the bottom sheet stays open and shows the information reactively, which is what we want.

@uhthomas uhthomas force-pushed the uhthomas/mobile-feat-inline-asset-details branch from c230c9c to 9d6f41a Compare February 8, 2026 23:07
@uhthomas
Copy link
Collaborator Author

uhthomas commented Feb 8, 2026

Thanks for testing the changes!

  1. I did this on purpose. It looks great with images, but horrible when there's text content. See below an example with/without when the bottom sheet is behind the system bar. Maybe it's platform specific? It could be fine on iOS? This is also what Google Photos does fwiw.
image image
  1. I think that's fair? I've pushed an update to address that. It's not quite as nice as Google Photos, as the bottom sheet doesn't move with the image. This is because the page view is much further down. It would need a much bigger refactor to get the bottom sheet and image in sync horizontally I think.

@uhthomas
Copy link
Collaborator Author

uhthomas commented Feb 8, 2026

One thing which might be nice - it currently hard stops when scrolling to the bottom, a bouncy / springy scroll would feel a bit better .

@mertalev
Copy link
Member

mertalev commented Feb 9, 2026

Apple Photos fullscreens and hides the time etc. when the bottom sheet is opened. I think that's the nicest behavior

@uhthomas
Copy link
Collaborator Author

uhthomas commented Feb 9, 2026

I'm open to trying that. I probably won't be able to take a look until tomorrow evening though.

@alextran1502
Copy link
Member

I did this on purpose. It looks great with images, but horrible when there's text content.

I would prioritize making it look great for images. I'd like to keep it as is on main

as the bottom sheet doesn't move with the image

I don't think we need the bottomsheet move with the images, as long as the bottomsheet isn't dismissed and the data is updated, then I am fine with that 😄

@uhthomas
Copy link
Collaborator Author

uhthomas commented Feb 9, 2026

The behaviour on main is a bit different, because the bottom sheet doesn't go behind the status bar and scrolls differently. We could meet in the middle and make the black/white bar transparent? Does that work for you? Or does Mert's suggestion of making it full screen sound better?

@uhthomas uhthomas force-pushed the uhthomas/mobile-feat-inline-asset-details branch from 9d6f41a to 5dd6d5f Compare February 9, 2026 20:01
@uhthomas
Copy link
Collaborator Author

uhthomas commented Feb 9, 2026

I've updated the PR to hide the system UI when the asset details are showing. Let me know what you think.

@uhthomas uhthomas force-pushed the uhthomas/mobile-feat-inline-asset-details branch 5 times, most recently from 47f27e5 to 932e3a1 Compare February 10, 2026 00:02
@uhthomas
Copy link
Collaborator Author

Per discussion in discord, hiding the system UI on Android makes the app very laggy. This doesn't seem to be the case on iOS, so we are going to hide the system UI on iOS, and use a semi-transparent black box behind the system bar on Android.

@uhthomas uhthomas force-pushed the uhthomas/mobile-feat-inline-asset-details branch 6 times, most recently from 329077b to 76704a2 Compare February 10, 2026 02:16
The existing implementation for showing asset details uses a bottom
sheet, and is not in sync with the preview or scroll intent. Other apps
use inline details, which is much cleaner and feels better to use.
@uhthomas uhthomas force-pushed the uhthomas/mobile-feat-inline-asset-details branch from 76704a2 to f87581b Compare February 10, 2026 02:22
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.

3 participants