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

[Update] Download preplanned map area feedback #258

Merged
merged 8 commits into from
Sep 11, 2023

Conversation

des12437
Copy link
Contributor

@des12437 des12437 commented Aug 23, 2023

Description

This PR addresses feedback from Nick on the initial testing for 200.1.0 sample viewer TestFlight app for the Download preplanned map area sample:

  • Oooh. A legit bug! I dismissed the preplanned area picker in the preplanned sample then navigated back from the preplanned sample to the samples list and the preplanned area picker reappeared over the sample list! I think you have to download a preplanned area to see this.
    Might be related to the sheet issue as well: Fix sample info view presentation auto dismiss when custom sheet modifier is in place #80

    This PR fixes this bug caused by a sheet selection bug. Previously when the "Done" button was selected the dismiss action did not set the isSelected property to false.

  • This map ("Seager Park Area") looks empty by default on an iPhone. I have to zoom in to notice it has content. Not sure what to do about that though…
    I spotted the same when I review this sample. The best way IMO is to set the initial zoom scale on these preplanned map area to a close enough value so the symbols appear on all platform regardless desktop or mobile.

    This issue is fixed by expanding the visible area's envelope when an offline map is selected. Now all layer content is visible for each offline map.

  • Would it be better if, when you download an area, it is automatically selected? Currently you have to tap to download, wait for it to download, and then tap it again to select it.

    I did not implement this suggestion based on discussion in the swift issue deciding to not make this change.

Linked Issue(s)

  • swift/issues/3931

How To Test

  • View each offline map after downloading and verify that all layer content is visible.

Screenshots

Before/After layer visibility

@des12437 des12437 requested a review from yo1995 August 23, 2023 19:37
@des12437 des12437 self-assigned this Aug 23, 2023
@yo1995 yo1995 requested review from a team, zkline101 and nixta and removed request for a team August 23, 2023 23:58
@des12437 des12437 requested a review from yo1995 August 24, 2023 21:48
@des12437 des12437 requested review from yo1995 and removed request for zkline101 September 6, 2023 20:41
@nixta
Copy link
Member

nixta commented Sep 8, 2023

If I open the on-demand sample then the preplanned sample, I don't see any controls below the map.

Oddly, switching between samples got the controls to show up once, but now I can't get them to show up at all.

IMG_F20F6EA14C4B-1

@des12437
Copy link
Contributor Author

des12437 commented Sep 8, 2023

If I open the on-demand sample then the preplanned sample, I don't see any controls below the map.

Oddly, switching between samples got the controls to show up once, but now I can't get them to show up at all.

I tried to reproduce this on iOS 16 and 17 but could not. What iOS version and simulator did you see this issue on? Also which on-demand sample did you open first?

@nixta
Copy link
Member

nixta commented Sep 8, 2023

It's really inconsistent. I got unlucky and saw it first time I ran the app, but after that it took quite some effort to see it again. Just search for "Offline" then switch between Generate and Preplanned. Here's a video. I switched back and forth a lot before I could repro for the video, and you'll notice that I can just exit the sample and go right back in and it works fine again.

iOS 16.6 on an iPhone 14 Pro device.

RPReplay_Final1694185607.mov

Sorry I can't get a better repro!

Copy link
Member

@nixta nixta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the weirdness of the bottom UX not appearing on the odd occasion, this looks much better. Thank you!

I'll let someone else make the call about whether that should be a show-stopper. Given how hard it's been for me to repro, I'd say probably not. We could open another issue for that.

@des12437
Copy link
Contributor Author

Aside from the weirdness of the bottom UX not appearing on the odd occasion, this looks much better. Thank you!

I'll let someone else make the call about whether that should be a show-stopper. Given how hard it's been for me to repro, I'd say probably not. We could open another issue for that.

I will log a separate issue for the UI bug, thank you for finding that!

@des12437 des12437 merged commit 2d5fb61 into v.next Sep 11, 2023
1 check passed
@des12437 des12437 deleted the des12437/Download-preplanned-map-area-feedback branch September 11, 2023 16:29
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.

None yet

3 participants