Esri / arcgis-maps-sdk-swift-toolkit

Mapping components that will simplify your Swift app development with the ArcGIS Maps SDK for Swift.
https://developers.arcgis.com/swift
Apache License 2.0
25 stars 7 forks source link

pr feedback: list-preplanned-map-areas #734

Closed rolson closed 1 month ago

rolson commented 1 month ago

This is my first round of feedback for PR. I see several issues with the code in List-Preplanned-Map-Areas.

This PR addresses several things, but the main issue I see is that status and download action are combined. That means that with the current code, I don't see a way that a download can fail, we can express that to the user, and they can try again. The status needs to be separate from the download button.

Other things it cleans up:

image

There is more feedback that I have that revolves around ambiguous status states and errors. Also the model needs to contain the AreaStatus because the view should simply be a function of the model. The view is doing a bit too much there, IMO.