getodk / collect

ODK Collect is an Android app for filling out forms. It's been used to collect billions of data points in challenging environments around the world. Contribute and make the world a better place! ✨📋✨
https://docs.getodk.org/collect-intro
Other
702 stars 1.35k forks source link

Revise progress UI in offline maps #6198

Open seadowg opened 3 weeks ago

seadowg commented 3 weeks ago

The current implementation displays the progress indicator in the center of the "Layers" section.

I'm thinking we should either show the progress bar at the top or show the circular indicator in a container with text to explain what's happening. I prefer option 1 because it's more clear.

Image

Figma

Notes

Once we've decided on a design, we should look at a restructure of how we show layers. In the current form, we need to share data using view models, but it probably makes more sense to introduce a LayersDataService that handles most of this for us.

Additionally, we probably want to hold the URIs the user imports in Fragment args rather than in a view model to support import cases where the app is killed in the background (while multitasking) and then returned to (this currently just closes the import screen).

alyblenkin commented 3 weeks ago

Updated this one @seadowg. Let me know what you think.

seadowg commented 2 weeks ago

In both cases we need to disable all the controls right? I'm worried it might be confusing to have disabled stuff on screen while copying the files.

alyblenkin commented 2 weeks ago

In both cases we need to disable all the controls right? I'm worried it might be confusing to have disabled stuff on screen while copying the files.

I think you would only have to disable "Add layers" but they can still cancel or select layer access while it's importing, right? I agree that could be confusing if we had to disable both bottom actions. We can also leave it as is. This was a nice to have.

seadowg commented 2 weeks ago

I think you would only have to disable "Add layers" but they can still cancel or select layer access while it's importing, right?

Yeah, that sounds right! I definitely like the direction of 1 more than 2. I'm not sure about the "card" like background, but it could be that I've just never seen it used like that before.

alyblenkin commented 2 weeks ago

I swear they had an image of it in the docs just last week! They do mention that progress indicators can be displayed in a card too, but they no longer have a reference (maybe I'm going mad). Regardless, I don't think it's necessary. The main thing is having a little descriptor so the user knows what's happening.

seadowg commented 12 hours ago

They do mention that progress indicators can be displayed in a card too, but they no longer have a reference (maybe I'm going mad). Regardless, I don't think it's necessary. The main thing is having a little descriptor so the user knows what's happening.

I think the weird part about the card for me is that we're not then going to show the layers in a card. Maybe we could just show a circular progress indicator in the space where the layers will be?

alyblenkin commented 6 hours ago

Maybe we could just show a circular progress indicator in the space where the layers will be?

That works for me. I think it would still be helpful to have the label "Importing layers" below the progress indicator just in case it takes a while.

lognaturel commented 6 hours ago

I’ve been wondering why that step takes so long. It’s not doing any copying, it’s just getting the layer names. Shouldn’t that be close to instant?

seadowg commented 5 hours ago

I’ve been wondering why that step takes so long. It’s not doing any copying, it’s just getting the layer names. Shouldn’t that be close to instant?

It is copying it into a cache. Correct me if I'm wrong here @grzesiek2010, but my understanding was that we're doing that so we can inspect the file metadata (to show the tile names). Unless there's another reason, I am now wondering if we could safely do this via the Uri Android gives us (via the Kotlin extension Uri#toFile that I've just spotted) rather than performing the extra copy into our cache.