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

Add area download FAB #145

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add area download FAB #145

wants to merge 2 commits into from

Conversation

wang-li
Copy link
Collaborator

@wang-li wang-li commented Jun 27, 2024

In order to ease the download of the photos for an area, an additional FAB has been added. This FAB will appear when an area is focused on the map. A click on it will display a bottom sheet to manage the download of the current area, along with the nearby ones.

area_download_bottom_sheet.mp4

@nmondollot
Copy link
Member

Great!

I found 2 bugs however:

1/ I go in "Franchard Sablons" and tap on the FAB and download the area
Then I move the map to "Hautes Plaines".
The area bar is flashing like crazy between the two areas

IMG_9998.MOV

2/ I go in "Franchard Sablons" and tap on the FAB and download the area
Then I move the map to "Franchard Cuisinière"
=> I can't see the status of the download when I tap on the FAB

Bug 1 is really annoying, let's fix it before we ship?
Bug 2 is more of a UI/UX concern, and I'm working on a cluster detector on iOS that may fix the problem => let's talk about it soon.

@wang-li wang-li force-pushed the feat/add-area-download-fab branch 3 times, most recently from 4185107 to cebb98d Compare August 1, 2024 14:47
@wang-li wang-li force-pushed the feat/add-area-download-fab branch from cebb98d to d5ee023 Compare August 7, 2024 14:38
@wang-li
Copy link
Collaborator Author

wang-li commented Aug 12, 2024

@nmondollot Here is a demo of the downloads on the map, handling the clusters

cluster_download_2.mp4

@nmondollot
Copy link
Member

Great!
It's working pretty well 🤩

My main feedback:

1/ the downloads are significantly slower than on iOS, why?
is there a way you can make more photo downloads in parallel? FYI our CDN (amazon cloudfront) can handle a lot of concurrent http requests in parallel, there is no risk of DoS
it's important to make it as fast as possible (eg. < 30s for a whole cluster on a fiber connection), otherwise users won't do it and it defeats the whole purpose of the feature

2/ when I try to download a whole cluster, I find it hard to understand the download progress because the spinners are all moving at different speeds
can you make it easier to see the progress? for instance by making the spinners still (like on iOS), or displaying the progress as text, or some other solution

3/ on iOS I decided to always show the fab button on the map, to make the UI more consistent
when there is no cluster selected I display a message inviting users to zoom on the map
what do you think?

4/ at first launch (no mapbox cache), when I tap on a cluster on the map (example: Buthiers) the fab button doesn't show up, probably because the cluster isn't selected properly
it might be because the mapbox data wasn't loaded when you infered the cluster?
FYI to mitigate this on iOS I introduced a dirty hack

Other feedback:

  • the "tous les secteurs" button looks exactly the same as the other buttons, which is a bit confusing => can we make it stand out more? (size, font, color, ...)
  • order of downloads : when user downloads a whole cluster we should do it in the order of the list (it's more intuitive)
  • when i download only 1 area, the cluster spinner should not be triggered
  • should we keep the download button on the area page? I removed it on iOS to streamline the UX, and to increase the chances that the user downloads the whole cluster and not just the area. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants