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

fix(mobile): appBar on home screen animates out and doesnt alter asset grid position #9026

Merged
merged 7 commits into from
Apr 25, 2024

Conversation

ConnerWithAnE
Copy link
Contributor

@ConnerWithAnE ConnerWithAnE commented Apr 22, 2024

Description

Fixes #9015

This alters the appBar in the home page. I removed the ternary if statement which was checking if multi-selection is enabled. I see the value in removing the bar to gain space but it does make the UI feel weird. Perhaps it could be a setting that can be added later if users want.

This alters the HomePage() widget return. The HomePage() is now a Stack which allows the MultiselectGrid() to scroll behind it. The page no longer shifts when an asset is selected and the ImmichAppBar() animates out to the top as show in the example given by @waclaw66. This only affects the home page as there is a check in the ImmichAssetGridView() which checks if the current tab is index 0 and the Route is one of the main tabs within the TabController. If both of those checks come up true padding is added to accommodate the AppBar adding some extra space. The RefreshIndicator() also has an offset added conditionally.

How Has This Been Tested?

I ran flutter test and had no issues, I also ran it on an android device. I am unable to test IOS but I have no reason to believe anything would be different on either OS. I checked all views which use the asset grid and only saw the changes take effect on the main page.

@waclaw66
Copy link
Contributor

Thanks for the PR. One remark, it would be nice if you could achieve to replace ImmichAppBar with a transparent bar to extend the surface height in the similar way as GPhotos does.

Screen_Recording_20240423_124122_Photos.mp4_out.mp4

@ConnerWithAnE
Copy link
Contributor Author

ConnerWithAnE commented Apr 24, 2024

Thanks for the PR. One remark, it would be nice if you could achieve to replace ImmichAppBar with a transparent bar to extend the surface height in the similar way as GPhotos does.
Screen_Recording_20240423_124122_Photos.mp4_out.mp4

Yea I could give that a go, might need to change the way that the homepage is displayed slightly. Could you possibly post a video example of how the top of the asset view is on google photos when selected? just so I can see how it is handled. I'd appreciate it thanks.

Edit: I think I got it looking pretty good actually, let me know what you think.

@ConnerWithAnE ConnerWithAnE changed the title fix(mobile): appBar on home screen no longer dissapears on selection fix(mobile): appBar on home screen animates out and doesnt alter asset grid position Apr 24, 2024
@waclaw66
Copy link
Contributor

waclaw66 commented Apr 24, 2024

Edit: I think I got it looking pretty good actually, let me know what you think.

It looks awesome now, exactly as I expected. Thanks!

Screen_Recording_20240424_093547.mp4_out.mp4

@benmccann
Copy link
Contributor

Could you fix the failing format job?

@ConnerWithAnE
Copy link
Contributor Author

ConnerWithAnE commented Apr 25, 2024

Yup, meant to get to it was just away today. First thing tomorrow I will!

Copy link
Contributor

@alextran1502 alextran1502 left a comment

Choose a reason for hiding this comment

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

Perfect! Thank you very much! Let's fix the format and have it merged

FYI, next time you can allow maintainer to push to the PR and we can help take care of these things

@alextran1502 alextran1502 merged commit a0d0392 into immich-app:main Apr 25, 2024
22 checks passed
@waclaw66
Copy link
Contributor

waclaw66 commented May 21, 2024

@ConnerWithAnE could you please adapt this PR to album viewer as well? There is the same issue with jumping asset grid. I've tried to do on my own, but haven't succeeded. Thanks!

@ConnerWithAnE
Copy link
Contributor Author

@ConnerWithAnE could you please adapt this PR to album viewer as well? There is the same issue with jumping asset grid. I've tried to do on my own, but haven't succeeded. Thanks!

#9741 This should fix it.

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.

Selecting on Android causes screen shift
4 participants