-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix for the rotation bug #429
Fix for the rotation bug #429
Conversation
puneet-pdx
commented
May 15, 2024
- Add logic to only initialize the Callout when the MapView is done with initial draw
- Move the onViewPointChanged listener from MapView to Callout to prevent unnecessary MapView recompositions.
- Move invocation of the Content lambda after all the MapViewEventHandlers and ViewPointHandlers are set
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.
looks good. Good to get the supporting logic out of the MapView body. One suggestion to consider.
{ | ||
this.content() | ||
// Convert the given location to a screen coordinate | ||
var calloutScreenCoordinate: ScreenCoordinate by remember { |
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.
possibly this could key on location
then it doesn't need to be assigned in the LaunchedEffect.
var calloutScreenCoordinate: ScreenCoordinate by remember { | |
var calloutScreenCoordinate: ScreenCoordinate by remember(location) { |
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.
I think this is not the key but the initial value for the calloutScreenCoordinate. we are using the changes to the state of calloutScreenCoordinate to trigger recomposition.
…nRotation # Conflicts: # toolkit/geoview-compose/src/main/java/com/arcgismaps/toolkit/geoviewcompose/Callout.kt