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

Add map function to apply style transition during style load #1174

Merged
merged 8 commits into from
Feb 23, 2022

Conversation

kiryldz
Copy link
Contributor

@kiryldz kiryldz commented Feb 22, 2022

Summary of changes

Introduce new function MapboxMap.setStyleTransition and deprecate Style.setStyleTransition.

Main idea is that we need to apply transition options in the correct moment of time during style is loading while our current API allows to actually operate on Style object when style is already fully loaded.

Please take a look at the visuals, what we do there is:

  • load first streets style with following options TransitionOptions.Builder().duration(0).build()
  • wait for 2 seconds and load dark style with TransitionOptions.Builder().duration(5000).delay(3000).build()

User impact (optional)

Following pseudo-code:

loadStyleUri(Style.MAPBOX_STREETS) { firstStyle ->
  firstStyle.styleTransition = TransitionOptions.Builder()
     .duration(0)
     .build()
}

should be replaced with:


loadStyleUri(
  styleUri = Style.MAPBOX_STREETS,
  styleTransitionOptions = TransitionOptions.Builder()
     .duration(0)
     .build()
)

Pull request checklist:

  • Briefly describe the changes in this PR.
  • Include before/after visuals or gifs if this PR includes visual changes.
Before After
incorrect.mp4
correct.mp4
  • Write tests for all new functionality. If tests were not written, please explain why.
  • Optimize code for java consumption (@JvmOverloads, @file:JvmName, etc).
  • Add example if relevant.
  • Document any changes to public APIs.
  • Run ./gradlew apiDump to update generated api files, if there's public API changes, otherwise the verify-kotlin-binary-compatibility CI will fail.
  • Apply changelog label ('breaking change', 'bug 🪲', 'build', 'docs', 'feature 🍏', 'performance ⚡', 'testing 💯') or use the label 'skip changelog'
  • Add an entry inside this element for inclusion in the mapbox-maps-android changelog: <changelog>Add overloaded MapboxMap.loadStyleUri, MapboxMap.loadStyleJson, MapboxMap.loadStyle with TransitionOptions parameter allowing to apply transition style being loaded.</changelog>.
  • If this PR is a v10.[version] release branch fix / enhancement, merge it to main firstly and then port to v10.[version] release branch.

Fixes: < Link to related issues that will be fixed by this pull request, if they exist >

PRs must be submitted under the terms of our Contributor License Agreement CLA.

@kiryldz kiryldz self-assigned this Feb 22, 2022
@ank27
Copy link
Contributor

ank27 commented Feb 22, 2022

Since we apply styleTransition to map now, does the same transition will be applied when switching between styles?

@kiryldz
Copy link
Contributor Author

kiryldz commented Feb 22, 2022

Since we apply styleTransition to map now, does the same transition will be applied when switching between styles?

Previously it was not applied at all but yes, I did even state in docs that user could set it once - and it will be applied to all style loads happening afterwards.

@kiryldz kiryldz marked this pull request as ready for review February 22, 2022 15:50
@kiryldz kiryldz requested a review from a team as a code owner February 22, 2022 15:50
@kiryldz
Copy link
Contributor Author

kiryldz commented Feb 23, 2022

Since we apply styleTransition to map now, does the same transition will be applied when switching between styles?

Previously it was not applied at all but yes, I did even state in docs that user could set it once - and it will be applied to all style loads happening afterwards.

@ank27 correction after deeper look - we do actually reset transition options for the next style, it's reflected in code now, PTAL

@kiryldz kiryldz requested a review from pengdev February 23, 2022 15:11
@kiryldz kiryldz changed the title Add map function to apply style transition Add map function to apply style transition during style load Feb 23, 2022
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@yunikkk yunikkk left a comment

Choose a reason for hiding this comment

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

1 nit

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@pengdev pengdev left a comment

Choose a reason for hiding this comment

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

LGTM % nits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants