[Update] Improve visibility of dynamic entities in sample

[des12437]

Description

This PR addresses specific feedback from Nick on the initial testing for 200.1.0 sample viewer TestFlight app to improve the visibility of the dynamic entities in the Add dynamic entity layer sample. The visibility of the dynamic entities is improved by adding content offsets to the MapView so that the dynamic entities are still visible when the sheet is open.

• Add dynamic entity layer

Can the sheet be made to cover less of the screen? Or praise the visible area offsets could be used to keep the dynamic entities in view while the sheet is open?

Linked Issue(s)

• swift/issues/3931

How To Test

• Toggle the Dynamic Entity Settings sheet and verify that there are dynamic entities in view.
• Interact with the sheet and observe changes to the dynamic entities.

Screenshots

Before/After

iPhone 14 Pro Max

iPad Pro

Mac Catalyst

To Discuss

• Would a sheet that covers less of the screen be a preferred fix? A sheet with presentation detents .presentationDetents([.fraction(0.40)]) does result in a shorter sheet on iPhone where the dynamic entities are visible, however, results in a full screen sheet on iPad and Mac Catalyst which covers the dynamic entities.

[yo1995]

I'm in favor of adjusting the detent fraction than using geometry reader and edge inset. Also I will assign someone else to review as I don't have strong opinion on this one.

[des12437]

I'm in favor of adjusting the detent fraction than using geometry reader and edge inset. Also I will assign someone else to review as I don't have strong opinion on this one.

I would prefer to adjust the sheet height, however, the height can only be adjusted in iOS 16+ using PresentationDetent fraction(_:) or height(_:). The sheet height could be adjusted once the minimum deployment target is iOS 16.

[philium]

If there is a better API starting with iOS 16, then we should use it for users on iOS 16:

var settingsSheetDetents: [Detent] {
    if #available(iOS 16, *) {
        // Use 'fraction(_:)' or 'height(_:)'.
    } else {
        return [.medium]
    }
}

FYI: There's another issue. When presented fullscreen, the sheet is not dismissible. It needs a dismiss button and could do with a title as well. The easiest way is by making the root of SettingsView a navigation view and then adding a navigation title and a close button to the navigation bar. The HIG has guidelines for where that button should go:

Position Done and Cancel buttons as people expect. Typically, a Done or Dismiss button belongs in a sheet’s top-right corner (in a left-to-right layout) or top-left corner (in a right-to-left layout). The Cancel button belongs in a sheet’s top-left (in a left-to-right layout) or top-right (in a right-to-left layout) corner.

[nixta]

It's definitely improved on iPhone :)

Is there no way to size the sheet for its content?

I wonder if any change is even necessary for iPad or Catalyst?

Seems like the ideal would be that on iOS where the sheet fully covers the map, that the sheet be only as tall as it needs to be, and that the sheet's height is used to set the edge insets.

[des12437]

I wonder if any change is even necessary for iPad or Catalyst?

No, no changes to the sheet are needed for iPad or Catalyst.

This commit adds a fraction detent to the presented sheet based on size classes for iPhone in portrait orientation and iPad split view on iOS 16 so that the dynamic entities are visible.

iPhone 14 Pro Max

iPad Pro

Mac Catalyst

This implementation is more complex than this suggestion since the previously used sheet is a Toolkit sheet where Detent is an enumeration, so there is no way to pass a fraction() detent to that sheet. A regular sheet covers the whole screen on iPad and Mac Catalyst which is not ideal, so I used size classes. Should a dismiss button still be added to the sheet @philium?

[nixta]

Panel looks great on iPhone.

For iPad and Mac Catalyst, should there be great blank areas at the bottom of the popover? I'm surprised SwiftUI doesn't default to "the right size for the content", so perhaps it isn't possible.

[des12437]

For iPad and Mac Catalyst, should there be great blank areas at the bottom of the popover? I'm surprised SwiftUI doesn't default to "the right size for the content", so perhaps it isn't possible.

Sheets do not seem to be adaptive to their content since they have a .large detent size by default, and the other detent options are also fixed sizes.

Instead of adding an implementation in this sample for iOS 16 it may be better to support iOS 16 in View+Sheet.swift with the ability to set .fraction(_:) and height(_:) detents, however, that implementation could cause some refactoring in more than one sample if the model of the class changes.

[philium]

@des12437 Is this ready for re-review?

[des12437]

@des12437 Is this ready for re-review?

Yes, this PR is ready for re-review.

[philium]

When presented fullscreen, the sheet is not dismissible.

This is also an issue on macOS:

Please fix. As I mentioned, the easiest way is by using a navigation view as the root view of the settings view.

[des12437]

This PR is ready for review. The settings sheet now presents like so on the following devices:

iPad Pro:

iPhone:

Mac Catalyst:

[nixta]

Looks much better, thank you.