-
Notifications
You must be signed in to change notification settings - Fork 35
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 to use GeoviewCompose - SceneView #191
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments. Samples run well.
...in/java/com/esri/arcgismaps/sample/showviewshedfrompointinscene/components/SceneViewModel.kt
Outdated
Show resolved
Hide resolved
.../src/main/java/com/esri/arcgismaps/sample/showviewshedfrompointinscene/screens/MainScreen.kt
Show resolved
Hide resolved
...ain/java/com/esri/arcgismaps/sample/displayscenefrommobilescenepackage/screens/MainScreen.kt
Show resolved
Hide resolved
...a/com/esri/arcgismaps/sample/displayscenefrommobilescenepackage/components/SceneViewModel.kt
Show resolved
Hide resolved
...on/src/main/java/com/esri/arcgismaps/sample/addscenelayerwithelevation/screens/MainScreen.kt
Show resolved
Hide resolved
Thanks, @ruiqima, I addressed the feedback. Ready for your review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham7109 Looks good, one minor comment about the extra line.
Other comment:
-
For the
show-viewshed-from-point-in-scene
sample, I noticed a typo inintialSliderValue
in theViewshedSlider.kt
file, should beinitialSliderValue
since we are making changes to this file, it will be good to address this minor typo as well. -
In the same file, (ignore this comment, I see its been addressed here)
var sliderValue by remember {
mutableStateOf(intialSliderValue)
}
could be modified to
var sliderValue by remember {
mutableFloatStateOf(intialSliderValue)
}
.../src/main/java/com/esri/arcgismaps/sample/showviewshedfrompointinscene/screens/MainScreen.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Description
PR to update the 3 missed SceneView compose samples to use geoview-compose SceneView:
Links and Data
Sample issue:
runtime/kotlin/issues/3776