Update show coordinates sample #152
Conversation
There was a problem hiding this comment.
@prupani-7 Looks good and works as expected, a couple small comments
| import com.esri.arcgismaps.sample.showcoordinatesinmultipleformats.components.MapViewModel | ||
|
|
||
| /** | ||
| * Main screen layout for the sample app | ||
| */ | ||
| @Composable | ||
| fun MainScreen(sampleName: String) { | ||
| fun MainScreen(sampleName: String, application: Application) { |
There was a problem hiding this comment.
Does this need to be passed in? Could you use LocalContext.current.applicationContext?
| val arcGISMap = ArcGISMap(Basemap(basemapLayer)) | ||
| // the collection of graphics overlays used by the MapView | ||
| val graphicsOverlayCollection = rememberGraphicsOverlayCollection() | ||
| // the collection of graphics overlays used by the MapView |
There was a problem hiding this comment.
This looks like a duplicate comment to line 55
|
|
||
| Scaffold( | ||
| topBar = { SampleTopAppBar(title = sampleName) }, | ||
| content = { | ||
| content = { it -> |
There was a problem hiding this comment.
Was this intentional? I expect it to be named or not declared explicitly
|
@hud10837 thanks for the feedback! addressed them, re-review? |
There was a problem hiding this comment.
The graphic point does not update when I try the reverse workflow.
- Tapping a point elsewhere
- Pasting in a predeterminate coordinate
- All text fields update to the new coordinate as expected
- The graphic point on map does not
Could you debug to see if this issue is due to the toolkit MapView or the sample workflow?
There was a problem hiding this comment.
good catch, fixed...
the geometry of the coordinateLocationGraphic was not getting updated to the new Point.
| graphicsOverlays = graphicsOverlayCollection.apply { | ||
| this.add(graphicsOverlay) | ||
| }, |
There was a problem hiding this comment.
I think simple assignment properties here would be more clear, and we can move the apply block away from the Compose call and add it to the line the val is declared.
// graphics overlay for the MapView to draw the graphics
val graphicsOverlay = remember { GraphicsOverlay() }
// the collection of graphics overlays used by the MapView
val graphicsOverlayCollection = rememberGraphicsOverlayCollection().apply {
add(graphicsOverlay)
}| graphicsOverlays = graphicsOverlayCollection.apply { | |
| this.add(graphicsOverlay) | |
| }, | |
| graphicsOverlays = graphicsOverlayCollection, |
There was a problem hiding this comment.
This solution fixed the issue with showMessageDialog not showing and app crashing with error (GO object already owned)
I think everytime MapView composable was called, we were adding the same GO object to the GO collection, without clearing it.
but moving the apply block in declaration fixes this. Thanks!
| /** | ||
| * Updates the tapped graphic and coordinate notations using the [tappedPoint] | ||
| */ |
There was a problem hiding this comment.
| /** | |
| * Updates the tapped graphic and coordinate notations using the [tappedPoint] | |
| */ | |
| // retrieve the map point on MapView tapped |
| // set up a graphic to indicate where the coordinates relate to, with an initial location | ||
| val initialPoint = Point(0.0, 0.0, SpatialReference.wgs84()) | ||
|
|
||
| val coordinateLocation = Graphic( |
There was a problem hiding this comment.
coordinateLocation might be confused with an actual location point.
Could rename it to coordinateLocationGraphic
| val coordinateLocation = Graphic( | |
| val coordinateLocationGraphic = Graphic( |
There was a problem hiding this comment.
The app crashes when I type in an incorrect coordinate point. Ideally, it should display the messageDialogVM.showMessageDialog
|
@shubham7109 thanks for the feedback, applied your suggestions, please re-review? |
shubham7109
changed the title
Description
PR to update the sample to use composable mapview from the toolkit
Versions.gradle file from PR uses "arcgisToolkitVersion = '200.3.0-1'"
So, update new module script PR should be merged before this PR
Links and Data
Sample Epic:
runtime/kotlin/issues/3368What To Review
How to Test
Run the sample on the sample viewer or the repo.