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

Feature/android support #2

Merged
merged 9 commits into from
Nov 3, 2017
Merged

Feature/android support #2

merged 9 commits into from
Nov 3, 2017

Conversation

mcousillas6
Copy link
Contributor

@mcousillas6 mcousillas6 commented Nov 2, 2017

Added android support using the LineAndroid SDK.

public class LineLogin extends ReactContextBaseJavaModule {
private static final String MODULE_NAME = "LineLoginManager";
private static final String ERROR = "ERROR";
private static final String CHANNEL_ID = "1544017747";
Copy link
Member

Choose a reason for hiding this comment

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

as you said, this must be received as parameter


@ReactMethod
public void loginWithPermissions(final Promise promise) {
login(promise);
Copy link
Member

Choose a reason for hiding this comment

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

this way, it looks you are not passing the permissions to the Line login.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not, the problem here is that the android login does not accept any kind of parameters to set permissions. I did this just to match the iOS api that has that feature.

include ':react-native-line'
project(':react-native-line').projectDir = new File(rootProject.projectDir, '../node_modules/react-native-line/android')

include ':app'
Copy link
Member

Choose a reason for hiding this comment

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

missing new line

@@ -27,7 +27,7 @@ class example extends Component {
}

_handleClickLogout () {
LoginManager.logout()
// LoginManager.logout()
Copy link
Member

Choose a reason for hiding this comment

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

why is this commented? can be deleted?

loginResult = LineLoginApi.getLoginResultFromIntent(data);
switch (loginResult.getResponseCode()) {
case SUCCESS:
loginPromise.resolve(loginResult.getLineCredential().getAccessToken().getAccessToken());
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem to be compatible with iOS. On iOS the result is sent as a dictionary with two keys: accessToken and profile.

@ReactMethod
public void logout(final Promise promise) {
loginResult = null;
promise.resolve(new Object());
Copy link
Member

Choose a reason for hiding this comment

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

To correctly logout the user from Line you need to call to LineApiClient.logout


@ReactMethod
public void getUserProfile(final Promise promise) {
if (loginResult != null) {
Copy link
Member

Choose a reason for hiding this comment

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

I think that a better approach would be using the LineApiClient to get the actual user profile. The current approach will work only if you called the login function first and it will return a cached version of the profile. Thoughts?


@ReactMethod
public void currentAccessToken(final Promise promise) {
if (loginResult != null) {
Copy link
Member

Choose a reason for hiding this comment

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

@@ -22,7 +23,8 @@ public boolean getUseDeveloperSupport() {
@Override
protected List<ReactPackage> getPackages() {
return Arrays.<ReactPackage>asList(
new MainReactPackage()
new MainReactPackage(),
new LineLoginPackage()
Copy link
Member

Choose a reason for hiding this comment

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

please fix indentation

private LineLoginResult loginResult;
private final ActivityEventListener mActivityEventListener = new BaseActivityEventListener() {
@Override
public void onActivityResult(Activity activity, int requestCode, int resultCode, Intent data) {
super.onActivityResult(activity, requestCode, resultCode, data);
if (requestCode != REQUEST_CODE) {
loginPromise.reject(ERROR, "Unsupported request");
currentPromise.reject(ERROR, "Unsupported request");
Copy link
Member

Choose a reason for hiding this comment

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

don't you need to check if currentPromise is null here too?

}

private LineApiClient getLineApiClient() {
if(lineApiClient == null) {
Copy link
Member

Choose a reason for hiding this comment

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

please add space after if

loginPromise = promise;
Intent intent = LineLoginApi.getLoginIntent(getCurrentActivity().getApplicationContext(), CHANNEL_ID);
currentPromise = promise;
Context context= getCurrentActivity().getApplicationContext();
Copy link
Member

Choose a reason for hiding this comment

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

please add space before =

}

public class GetAccessTokenTask extends AsyncTask<Void, Void, LineApiResponse<LineAccessToken>> {
final static String TAG = "GetAccessTokenTask";
Copy link
Member

Choose a reason for hiding this comment

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

are the TAGs being used?

@@ -1,3 +1,4 @@
<resources>
<string name="app_name">example</string>
<string name="line_channel_id">1544017747</string>
Copy link
Member

Choose a reason for hiding this comment

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

please don't push this change

@@ -1,3 +1,4 @@
<resources>
<string name="app_name">LineLoginManager</string>
<string name="line_channel_id">Your channel id here</string>
Copy link
Member

Choose a reason for hiding this comment

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

please add attribute translatable="false" here and bellow

@mcousillas6 mcousillas6 merged commit b5332df into master Nov 3, 2017
@mcousillas6 mcousillas6 deleted the feature/androidSupport branch November 3, 2017 16:37
BondPiyapan pushed a commit to BondPiyapan/react-native-line-ex that referenced this pull request May 23, 2022
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.

2 participants