Esri / data-collection-ios

Mobile data collection app using the iOS Runtime SDK.
https://developers.arcgis.com/
Apache License 2.0
25 stars 26 forks source link

Small pop-up view touch-drag event dismiss behavior #192

Closed esreli closed 5 years ago

esreli commented 5 years ago

The lack of this feature has bugged me for a while. Before now, to dismiss a currently selected pop-up, a user would have to select an area of the map where there are no identifiable features. Now the user can touch-drag up or down past a threshold (of 60 points) to dismiss the small pop-up view.

Asking for review from @philium and @nixta for code quality and from @mikewilburn for usability. Of course, these review delineations can blur, if you like.

philium commented 5 years ago

Just some usability issues I found while playing around with this feature:

image

esreli commented 5 years ago

@philium thanks for your feedback.

Both dragging up and dragging down causes the view to be dismissed. Consider only dismissing when dragged down and animating back to the original position when dragged up, similar to the new drag-to-dismiss feature in iOS 13.

Sounds good, i'll implement.

If drag gesture doesn't reach the threshold to dismiss, the tap action is performed instead. Consider simply animating the view back to the original position as the user may have initiated a drag by mistake.

I'll make the threshold smaller.

If the app is dismissed while dragging, the view stays where the user dragged i

I'll fix.

esreli commented 5 years ago

@philium would you have another look?

philium commented 5 years ago

All of the aforementioned issues appear to be resolved. I did find another minor issue, which is that if the device is rotated while dragging, the view stays where the user dragged it.

mikewilburn commented 5 years ago

I've tested this with the latest changes and don't have any unwanted observations to report. LGTM 👍 .

esreli commented 5 years ago

@philium I believe all concerns have been addressed through a more unified approach, in this latest commit. Would you re-review?