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(access-token): match iOS interface for getAccessToken() on Android #954

Merged
merged 1 commit into from
Jul 31, 2020
Merged

fix(access-token): match iOS interface for getAccessToken() on Android #954

merged 1 commit into from
Jul 31, 2020

Conversation

bllanos
Copy link
Contributor

@bllanos bllanos commented Jul 13, 2020

This pull request addresses issue #937

I replicated the behaviour of getAccessToken() on iOS in the native Android code. Both iOS and Android versions now return a promise that either resolves to a string, or is rejected if the access token is null. Previously, the Android version returned a promise that resolved to an object with an accessToken property having the value of the access token.

This is a breaking change, changing the interface of getAccessToken() on Android.

Testing performed

I completed manual testing from a basic app similar to the getting started example, not knowing how or if I should add tests for native Android code:

  • getAccessToken() returns a string equal to the access token set previously through setAccessToken().
  • getAccessToken() rejects the promise with an error message if null was previously set as the access token through setAccessToken().

I tested the basic app on an Android emulator (API level 28) and on a physical device (Google Pixel, Android 10 build number QP1A.191005.007.A3). The client app uses React Native Version 0.62.2.

I do not have an iOS development environment to test with.

@ferdicus ferdicus added this to the Release 8.1 milestone Jul 24, 2020
Copy link
Member

@ferdicus ferdicus 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 to me 👍

@ferdicus
Copy link
Member

@mfazekas, any objections?
I would open a PR with additional docs for MapboxGL methods, and we can tag it as a breaking change for the upcoming release?

@ferdicus
Copy link
Member

@mfazekas
Copy link
Contributor

@ferdicus we should document in the changelog i think that is enough.

I think there could be argument made that it's a breaking change, but it could be considered a bug fix as well. I'd vote for the bug fix.

@ferdicus ferdicus changed the title Match iOS interface for getAccessToken() on Android fix(access-token): match iOS interface for getAccessToken() on Android Jul 31, 2020
@ferdicus ferdicus merged commit 23cc594 into rnmapbox:master Jul 31, 2020
ferdicus referenced this pull request Jul 31, 2020
Add line for this PR [fix(access-token): match iOS interface for getAccessToken() on Android](https://github.com/react-native-mapbox-gl/maps/pull/954)
@bllanos bllanos deleted the fix/getaccesstoken-return-value branch July 31, 2020 14:19
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.

3 participants