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

refactor(mobile): maplibre #6087

Merged
merged 12 commits into from
Jan 15, 2024
Merged

refactor(mobile): maplibre #6087

merged 12 commits into from
Jan 15, 2024

Conversation

shenlong-tanwen
Copy link
Member

@shenlong-tanwen shenlong-tanwen commented Jan 1, 2024

Changes made:

  • Replaces flutter_map with maplibre_gl which should hopefully solve the performance issues with rendering vectors

TODO:

  • Test it on a physical iOS device
  • Two-pane view in landscape
  • Fix location picker
  • Add zoom to current location
  • Add no assets in bounds placeholder
  • Tests
  • Check if this passes the F-droid build

Copy link

cloudflare-pages bot commented Jan 1, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: cae09f9
Status: ✅  Deploy successful!
Preview URL: https://cf24e34f.immich.pages.dev
Branch Preview URL: https://refactor-mobile-maplibre.immich.pages.dev

View logs

@alextran1502
Copy link
Contributor

alextran1502 commented Jan 5, 2024

@shenlong-tanwen Thank you for the PR. It works very well on an actual iOS device, the map tile is now loading instantly.

I notice a few issues

  • The pinch to zoom is a bit sensitive.

  • I got some error while scrolling around the home page

image

  • In the location picker, when switching to the view of selecting the location on the map, the map isn't rendered

@shenlong-tanwen shenlong-tanwen marked this pull request as ready for review January 7, 2024 10:16
@shenlong-tanwen
Copy link
Member Author

* The pinch to zoom is a bit sensitive.

Let me check if there is something we can do to improve it. We can take this as a non-blocker and can handle it in a separate PR though. Can you check if double tap to zoom suffers from the same issue as well? On android, both seem fine to me.

* I got some error while scrolling around the home page

I tried scrolling through the home page few times but cannot reproduce this. Is there anything I can do to reproduce it consistently?

* In the location picker, when switching to the view of selecting the location on the map, the map isn't rendered

This was not implemented then. I've added it back now

@shenlong-tanwen shenlong-tanwen changed the title refactor(wip): mobile maplibre refactor(mobile): maplibre Jan 7, 2024
@waclaw66
Copy link
Contributor

waclaw66 commented Jan 9, 2024

Do you plan to consolidate location pin image with the web version?

@shenlong-tanwen
Copy link
Member Author

Do you plan to consolidate location pin image with the web version?

The refactoring includes a new pic but this is still different than the web version though.

)

* Refactored get gps coords

* Use var for linter's sake, should handle errors better

* Cleanup

* Fix linter issues
@alextran1502 alextran1502 merged commit e6c0f0e into main Jan 15, 2024
21 checks passed
@alextran1502 alextran1502 deleted the refactor/mobile-maplibre branch January 15, 2024 15:26
@waclaw66
Copy link
Contributor

This is how position settings looks like on Samsung Galaxy S20 FE...

  • a tall gray bar at the top
  • bottom buttons are hidden under system navigation bar

Nepojmenované

@shenlong-tanwen
Copy link
Member Author

This is how position settings looks like on Samsung Galaxy S20 FE...
* a tall gray bar at the top
* bottom buttons are hidden under system navigation bar

I cannot reproduce this on my end. Can you please check if #6553 fixes it for you?

@waclaw66
Copy link
Contributor

I cannot reproduce this on my end. Can you please check if #6553 fixes it for you?

Will try tomorrow, thanks.

@waclaw66
Copy link
Contributor

Seems to be fixed with #6553.

Nepojmenované

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.

None yet

4 participants