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

SearchView modifiers shouldn't take a Binding. #71

Open
mhdostal opened this issue Jun 7, 2022 · 1 comment
Open

SearchView modifiers shouldn't take a Binding. #71

mhdostal opened this issue Jun 7, 2022 · 1 comment
Labels
enhancement New feature or request

Comments

@mhdostal
Copy link
Member

mhdostal commented Jun 7, 2022

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)
@philium
Copy link
Contributor

philium commented Jan 9, 2023

One thing to consider is whether any of those values should be passed as an initializer parameter rather than through a modifier. Are they part of the view's data, do they make up part of the definition of what a search view is? Does a user always need to specify any of those values? If so, an initializer parameter makes more sense. But if it is just configuring the view, something that doesn't need to be done, then a modifier makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants