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

Fix for the rotation bug #429

Merged

Conversation

puneet-pdx
Copy link
Collaborator

  • 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

Copy link
Collaborator

@sorenoid sorenoid left a 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 {
Copy link
Collaborator

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.

Suggested change
var calloutScreenCoordinate: ScreenCoordinate by remember {
var calloutScreenCoordinate: ScreenCoordinate by remember(location) {

Copy link
Collaborator Author

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.

@puneet-pdx puneet-pdx self-assigned this May 16, 2024
…nRotation

# Conflicts:
#	toolkit/geoview-compose/src/main/java/com/arcgismaps/toolkit/geoviewcompose/Callout.kt
@puneet-pdx puneet-pdx merged commit a94e38e into feature-branches/callout May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants