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 sample - query feature table #160

Merged
merged 6 commits into from
Dec 23, 2023

Conversation

prupani-7
Copy link

Description

PR to update the sample to use composable mapview from the toolkit

Links and Data

Sample Epic: runtime/kotlin/issues/3366

What To Review

Review updated files

How to Test

Run the sample on the sample viewer or the repo.

Copy link
Collaborator

@shubham7109 shubham7109 left a comment

Choose a reason for hiding this comment

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

Sample works well! Just a few minor changes to make.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Replace the application parameter from the MainScreen with the use of LocalContext.current.applicationContext

Comment on lines 72 to 73
// define a MapViewpointOperation to set the initial viewpoint and update it when the viewpoint changes
var mapViewpointOperation: MapViewpointOperation? by mutableStateOf(null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The mapViewpointOperation here does not need to be nullable, you could set it to the initial viewpoint. And remove it from the init block.

Suggested change
// define a MapViewpointOperation to set the initial viewpoint and update it when the viewpoint changes
var mapViewpointOperation: MapViewpointOperation? by mutableStateOf(null)
// define a mutable MapViewpointOperation and set the initial viewpoint
var mapViewpointOperation: MapViewpointOperation by mutableStateOf(
MapViewpointOperation.Set(usaViewpoint)
)

@@ -113,28 +124,13 @@ class MapViewModel(
// get the extent of the first feature in the result to zoom to
val envelope = feature.geometry?.extent
?: return@launch messageDialogVM.showMessageDialog("Error retrieving geometry extent")
// update the map view to set the viewpoint to the state geometry
mapViewState.stateGeometry = envelope
// update the MapViewpointOperation to set the geometry of the returned feature
Copy link
Collaborator

Choose a reason for hiding this comment

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

"set the geometry of the returned feature" doesn't sound right to me here. Maybe the comment could be:

Suggested change
// update the MapViewpointOperation to set the geometry of the returned feature
// update the viewpoint to the bounding geometry of the returned feature

*/
class MapViewState {
// map used to display the feature layer
var arcGISMap: ArcGISMap by mutableStateOf(ArcGISMap(BasemapStyle.ArcGISTopographic))

Copy link
Collaborator

Choose a reason for hiding this comment

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

Extra line here

@shubham7109 shubham7109 added the enhancement New feature or request label Dec 20, 2023
@prupani-7
Copy link
Author

@shubham7109 Applied the suggestions, thanks! a re-review?

Copy link
Collaborator

@changanxian changanxian left a comment

Choose a reason for hiding this comment

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

👍

@prupani-7 prupani-7 merged commit 911a6d6 into feature-branch/geo-compose Dec 23, 2023
1 check passed
@prupani-7 prupani-7 deleted the pri/query-feature-table branch December 23, 2023 01:28
prupani-7 added a commit that referenced this pull request Dec 27, 2023
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

Successfully merging this pull request may close these issues.

3 participants