Some things have become more defined since the original implementation:

• Preserve trailing whitespace
• @Binding and @State property wrappers should be placed on the line of the item they're wrapping

The component unit tests need to access online resources to run. An example of this is the Search unit tests, which generate a large amount of geocoding requests. It would be good to have a way to mock these network requests in order to speed up the tests and provide a consistent test experience.

See also #86, Swift 2354

Some tests call for an API Key:

    override func setUp() async throws {
        ArcGISRuntimeEnvironment.apiKey = APIKey("<#API Key#>")
    }

That modifies global state, but it's never cleaned up.

There should be a teardown block that cleans up the environment.

It might be good to have an APIKeyTestCase base class that had this functionality so that an API key only needs to be set in one place instead of multiple test classes.

Issue:

When tapping the Compass to rotate a SceneView back to North, any camera settings, such as pitch or roll, are lost. The issue was realized during implementation of the new compass component.

Done Criteria:

Tapping the compass in a SceneView where pitch or roll have been applied will not result in these values being lost.

Steps to Reproduce:

Add a Compass to a SceneView. Rotate the scene and change the pitch, then tap the Compass (to rotate back to North). The pitch setting has been reset.

Area of Interest:

In CompassViewModel.swift func resetHeading() is responsible for reoreinting the viewpoint to north. This would presumably be the method for any compass related changes to support resolving this issue.

cc @mhdostal

• Add velocity based detent selection to the floating panel

When dragging the handle up or down in Apple Maps, the speed at which you drag helps dictate whether the floating panel moves to the next detent. Move a little bit slowly does not; moving a little bit quickly, does. You could base the decision to go to the next detent on the position AND velocity of the drag.

Source

The tests for the toolkit are running very slowly. Even with some test disabled, they are taking me over 500 seconds to run:

Test Suite 'ArcGISToolkitTests.xctest' failed at 2022-07-13 15:18:30.653.
	 Executed 37 tests, with 1 failure (1 unexpected) in 507.281 (507.302) seconds
Test Suite 'Selected tests' failed at 2022-07-13 15:18:30.654.
	 Executed 37 tests, with 1 failure (1 unexpected) in 507.281 (507.303) seconds

Ideally these tests would take less than a few seconds as they are unit tests. The fact that they take so long will discourage developers from running them and the tests will fall into disrepair.

The Bookmarks example has code for using the Environment vertical and horizontal size classes. Those are not used in the example and should be removed.

    @Environment(\.horizontalSizeClass)
    private var horizontalSizeClass: UserInterfaceSizeClass?
    
    @Environment(\.verticalSizeClass)
    private var verticalSizeClass: UserInterfaceSizeClass?

Xcode 14 produces a purple runtime warning when using the Utility Network Trace tool

In order to better match the pattern of Apple's SwiftUI views, the BasemapGalleryViewModel should be made internal and it's initialization and use moved into the BasemapGallery. The client would then create a BasemapGallery, with initializers similar to what the BasemapGalleryViewModel has currently. The BasemapGallery would be responsible for instantiating and setting up the view model.

Any additional properties would be set on the BasemapGallery via modifiers.

There is currently no way to anchor Toolkit components to the top of a MapView or SceneView's attribution bar. This is helpful to provide a balanced look to component's placement as well as to move the component up, in concert with the attribution bar, when the bar is expanded.

Ref Swift 1397

There is an issue building the iOS Samples app with the Toolkit using Xcode 14.

Starting with Xcode 14, apple is deprecating the support support for bitcode and we have some build errors related to toolkit with bitcode enabled.

ld: '/Users/sara8187/Library/Developer/Xcode/DerivedData/arcgis-ios-sdk-samples-gfsomrmslqsjpnbnfyxlhzaxfjom/Build/Products/Debug-iphoneos/ArcGISToolkit.o' does not contain bitcode. You must rebuild it with bitcode enabled (Xcode setting ENABLE_BITCODE), obtain an updated library from the vendor, or disable bitcode for this target. file '/Users/sara8187/Library/Developer/Xcode/DerivedData/arcgis-ios-sdk-samples-gfsomrmslqsjpnbnfyxlhzaxfjom/Build/Products/Debug-iphoneos/ArcGISToolkit.o' for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

According to the release notes:

Unless our plan is to wait until Xcode 14 is released and have that be a minimum requirement, we'll need to continue to include bitcode, which means we'll need to fix whatever is causing the error.

In the Floor Filter sample, we have the site/facility selector open by default. This is fine on iPad because the map isn't covered by the picker, but on iPhone, the entire screen is covered which can be a bit disconcerting as all of the other samples open up to a visible map.

For example, the basemap gallery and bookmarks and not presented by default when the sample is opened. A user must take action to open those menus.

As mentioned in Swift PR 2548, the toolkit's extended conformance to Hashable, Equatable and Identifiable for UtilityElement here should be reviewed and edited or removed as "it doesn't match the documented semantics of the Equatable protocol".

It would probably be good to check all extended Utility* conformances.

Types under consideration:

• UtilityElement #160
• UtilityAssetType #168
• UtilityNamedTraceConfiguration #169
• UtilityNetwork #180
• UtilityTraceFunctionOutput #183
• UtilityTerminal #182

makePersistent() implies that it will return an authenticator based on Swift convention. We should change this function name. Maybe something like authenticator.persist(...)?

Add some basic documentation to the repository's README. At a minimum show the list of current components, which links to component-specific doc and example.

This is an interim measure until the Toolkit team finalizes a format for cross-platform toolkit component doc.

• Currently, when the Floor Filter is first opened and loading has not yet completed, the user is shown "No matches found". This can be improved upon by indicating to the user that loading is still in progress. As suggested by @zkline101 here.

In the SDK, ObservableObject conformance was removed from GeoModel.

Model objects need to be added to each of the examples that currently store the map/scene directly as a state object.

From a Samples PR review...

It is rare in SwiftUI for a modifier to take a binding because rarely do modifiers need to modify the passed in value. In the case of some of these (all of them?), the value cannot be modified, so it doesn't make any sense for the biding to take a binding. For example, isGeoViewNavigating(:). In those cases, the type of the parameter should be changed to the value type of the binding itself._

The modifiers in question, queryCenter, geoViewExtent, and isGeoViewNavigating all take bindings and should be considered for changing away from binding to the value type.

                SearchView(
                     sources: [locatorDataSource],
                     viewpoint: $searchResultViewpoint
                 )
                 .resultsOverlay(searchResultsOverlay)
                 .queryCenter($queryCenter)
                 .geoViewExtent($geoViewExtent)
                 .isGeoViewNavigating($isGeoViewNavigating)

This test is crashing for me: testSelectBookmarkWithModifierAndMap

The SearchField class is marked public, but the single initializer is only internal. It should be public.

Having the views presented via alert would likely be a more standard UX. (username/password prompt, certificate password prompt)

E.g. Replace this:

with something close to this:

Reference: Common Toolkit #202

In the Authentication example:

1. place an invalid username/password into the login form
2. Let the sign in operation fail
3. Restart the app,
4. The invalid username is now persisted as it's shown in the "Sign in with [username]" button

It is possible in the drag code to have the height of the floating panel be larger than the displayed height. This results in a delay when dragging up (making the floating panel smaller) until the calculated height is <= the displayed height.

See the comment here.

Code:

    var drag: some Gesture {
        DragGesture(minimumDistance: 0)
            .onChanged { value in
                handleColor = .activeHandleColor
                // Note:  There is a bug here where `height` can be set
                // larger than the displayed height.  This occurs by continuing
                // to drag down on the handle after the panel reaches it's max
                // height.  When that happens subsequent "drag up" operations
                // don't cause the panel to shrink immediately, but will
                // ultimately snap to the correct height.
                height = max(.minHeight, (height ?? 0) + value.translation.height)
            }
            .onEnded { _ in
                handleColor = .defaultHandleColor
            }
    }

In order to better match the pattern of Apple's SwiftUI views, the SearchViewModel should be made internal and it's initialization and use moved into the SearchView. The client would then create a SearchView, with initializers similar to what the SearchViewModel has currently. The SearchView would be responsible for instantiating and setting up the view model.

Any additional properties would be set on the SearchView via modifiers.

Remove deprecated basemap methods.

Toolkit-common issue

Currently, most of the maps/scenes used in the examples are defined as follows:

private let map = Map(...)

To minimize the cost of view re-initialization, they should be defined as @StateObjects:

@StateObject private var map = Map(...)

This would apply to any maps/scenes created in the examples, and probably in the OverviewMap.

The following are additional items to consider for the Floor Filter following initial feedback on FF_III PR #36 or in a potential new FF_IV PR.

• Improve on level name truncation.
• Experiment with increased width of the site/facility picker to avoid title truncation.
• In compact environments, switch the site/facility picker to be a sheet, (halfscreen preferred).
• Switch search bars to .searchable
• Change facility names to a common button style
• Improve tests
• Switch .navigationBarItems to .toolbar as it's deprecated
• Switchover to new sample map with fictional sites and facilites
• Switch level picker collapse button to a downward chevron
• All sites button: use a toolbar at the bottom to get the blurring. (Skipped for now due to SwiftUI bug)

It's been requested that the Compass have a default size that the user can override. This will simplify client code when adding the Compass.

• Tap "Sign In"
• Tap "Cancel"
• you will see this warning in the console:

=== AttributeGraph: cycle detected through attribute 103752 ===
=== AttributeGraph: cycle detected through attribute 103752 ===
=== AttributeGraph: cycle detected through attribute 104216 ===
=== AttributeGraph: cycle detected through attribute 104216 ===
=== AttributeGraph: cycle detected through attribute 103752 ===
=== AttributeGraph: cycle detected through attribute 103752 ===
=== AttributeGraph: cycle detected through attribute 103752 ===
=== AttributeGraph: cycle detected through attribute 104216 ===
=== AttributeGraph: cycle detected through attribute 104216 ===
=== AttributeGraph: cycle detected through attribute 103752 ===
2022-07-27 13:12:59.104218-0500 AuthenticationExample[10764:47490343] [SwiftUI] Modifying state during view update, this will cause undefined behavior.

It can be traced to this code:

            .toolbar {
                ToolbarItem(placement: .navigationBarTrailing) {
                    Button {
                        showProfile = true
                    } label: {
                        Image(systemName: "person.circle")
                    }
                }
            }

It's the .toolbar view modifier that's the issue.

I'm liking the Basemaps gallery. I have an idea for an enhancement that I think would be nice.

When you select a basemap for the first time, it does some loading, and it might determine that the basemap is not valid:

That's great.

After that, however, can we add a little red circle in the corner with a white exclamation mark to basically tell the user that we know this basemap is not valid?

Like this (but not cut off):

Further requested enhancements for the Floating Panel via the Apps Team:

• The floating panel should be able to specify the possible detents such that it can only be set to a certain size by an app user. E.g. presentationDetents(_:)
• Custom detent sizes. E.g. fraction(_:) or height(_:) #133
• Integrate scrolling and detent such that pulling downward from the top of scrollable content reduces the detent and pulling upward increases the detent.

Since the initial completion of the first few components, the best practices and coding style guidelines have evolved. This issue enumerates those changes that should be incorporated across all components.

• @State, @Binding, and other property wrappers should be on the same line as the variable declaration.

• View models should be internal and created by the main component view. Required model properties should be passed in as arguments to the view constructor, with additional properties set via view modifiers.

• Loading a geoModel before being passed to a component and loading a geomodel in a component should be standardized, including the display of loading progress and error(s).

List of components:

• BasemapGallery - PR
  • Associated SwiftAPI issue: #1311
• OverviewMap - PR
• Search
• Compass
• Floating Panel (?)

The search component will zoom to the extent of search results (after displaying the results as graphics in the MapView), but results will often be hidden by the search result list. Use the MapView.contentInset property to only display the results in the visible area of the MapView.

• use the .contentInset modifier of MapView
• only works with MapView; SceneView has no .contentInset modifier
• pass a binding to the contentInset to the SearchView
• the SearchView should set the contentInset, based on the area not covered by the result list and "repeat search here" button

The compass is documented here.

• Add link in the main readme here
• Add link in the documentation table of contents here

• Add the ability to create custom height detents for the floating panel

It would be nice if we had a custom "summary" detent height (or a "minimumHeight), to allow at least a minimum height needed to display a view's "summary" information.

Source

In some use cases, a component may use the .esriBorder() modifier but have a background color that is different from the modifier's default background color. This can result in an unwanted border effect.

In order to resolve this, the .esriBorder() modifier should take a background color parameter so that it can match that of the content it is modifying.

Originating Discussion

The design states that this initializer should be part of the pubic API, but it is currently marked internal. Why?

    /// Creates a `BasemapGallery` with the given geo model and portal.
    /// The portal will be used to retrieve basemaps.
    /// - Parameters:
    ///   - geoModel: A geo model.
    ///   - portal: The portal to use to load basemaps.
    internal init(_ geoModel: GeoModel? = nil, portal: Portal)

Update doc if this changes.

When I'm migrating an old sample to SwiftUI, I’d like to display a callout on the search result marker, and when the search bar text is cleared, also dismiss the callout. See this line: https://github.com/Esri/arcgis-runtime-samples-ios/blob/main/arcgis-ios-sdk-samples/Search/Find%20address/FindAddressViewController.swift#L134

Basically I need sth similar to the old delegate method searchBar(_:textDidChange:), to add actions triggered by text changed event.

Currently there isn't a way to listen to search field's events.

This is low priority.

In this feedback entry, @philium suggest that both EdgeInsets.toolkitDefault and View.esriBorder not be public as they're intended for toolkit components only.

View.esriBorder is currently used publicly in BasemapGalleryExampleView.

Should we or should we not move forward with this suggestion?

cc @mhdostal

An initial version of a Floating Panel has been created. The goal is to have a component similar in functionality to the Data Collection Open Source App floating panel. The original OSA floating panel design is below. The code can be found here.

This would include the following additional features:

• Header view to provide summary information and navigation support
• Integrate scrolling and detent such that pulling downward from the top of scrollable content reduces the detent and pulling upward increases the detent.
• Revise how the initial viewpoint is created in the example in response to this comment
• The ability to snap to or be locked on pre-defined heights for "summary", "half-height", "full height" while dragging. E.g. presentationDetents(_:)
• Custom detent sizes. E.g. fraction(_:) or height(_:) #133
• Size-class-specific location of handle: on the bottom of the panel for iPad, at the top for iPhone.
• Instead of the Floating Panel being a public view, expose a modifier for it. See the discussion [here].(#12 (review)) #108

The floating panel isn't really a floating panel unless it is in an overlay, right? Otherwise it could be presented in a popover or used in a navigation view. Instead of exposing it as a view, consider exposing it as a modifier, similar to how SwiftUI doesn't have a SheetView, but a modifier that presents a sheet.

The v100.x Toolkit had a SwiftLint build phase. We should add SwiftLint to the Toolkit, Example, and AuthenticatorExample projects (the Authenticator Example is essentially static and unchanging, so we're not going to lint that). The Samples team has already done this for the Swift Samples, so follow that pattern.

• Add to Example App via Run Script Build Phase
• Add to Toolkit Package via SwiftLint SPM Plug-in.

The FloorFilter component uses the following mechanism to display a selected row. This should be adopted in all list views in the Toolkit.

.listRowBackground(facility.id == viewModel.selectedFacility?.id ? Color.secondary.opacity(0.5) : Color.clear)

It would be great if adding an overview map were simpler. One way of doing that would be to have a custom view modifier for the overview map in the toolkit:

MapView(map: map)
    .overviewMap(alignment: .topTrailing)

A proof of concept is as follows:

extension MapView {
    func overviewMap(alignment: Alignment) -> some View {
        MapViewWithOverviewMap(mapView: self, alignment: alignment)
    }
}

struct MapViewWithOverviewMap: View {
    let mapView: MapView
    let alignment: Alignment
    
    @State private var viewpoint: Viewpoint?
    @State private var visibleArea: Polygon?
    
    var body: some View {
        mapView
            .onViewpointChanged(kind: .centerAndScale) { viewpoint = $0 }
            .onVisibleAreaChanged { visibleArea = $0 }
            .overlay(alignment: alignment) {
                OverviewMap.forMapView(
                    with: viewpoint,
                    visibleArea: visibleArea
                )
                .frame(width: 200, height: 132)
                .padding()
            }
    }
}

As a proof of concept, it isn't without problems. We would need to change how those modifiers on map/scene view are implemented since users may also want to use them themselves (currently each subsequent use would override the previous).

The overview map could be customized with an overviewMapStyle(_:) modifiers, similar to other SwiftUI views. This should be explored as part of this issue; the OverviewMap already provides modifiers for symbol, map, and scaleFactor. Should they be combined into one overviewMapStyle(map:, symbol:, scaleFactor:)?

To get around the .onViewpointChanged(kind:)/.onVisibleAreaChanged() issue, maybe the .overviewMap(alignment:) modifier could take an optional binding to a Viewpoint and visible area Polygon and then update it alongside the local viewpoint and visible area.

Thanks to @philium for the suggestion!

There were API changes made to the Compass component, but the tests were not updated.

cc @philium

In the Utility Network component, tapping on a selected starting point displays a list of the starting point's geoElement attributes.

Other platforms use popups to format this data for display.

Once popups are available in the Swift SDK, they should be used to improve the display of these attributes to the user.

Originating Discussion

Instead of the Floating Panel being a public view, expose a modifier for it. See the discussion [here].(#12 (review))

The floating panel isn't really a floating panel unless it is in an overlay, right? Otherwise it could be presented in a popover or used in a navigation view. Instead of exposing it as a view, consider exposing it as a modifier, similar to how SwiftUI doesn't have a SheetView, but a modifier that presents a sheet.

PR #70 rendered the existing compass tests unfeasible.

The existing tests were then removed in #87.

Should further changes be made to make the compass testable once again.