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

150817 adding on request permission result #2182

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

DurankGts
Copy link
Contributor

1-New method added in LyfeCycleListener and AndroidNativeUtils to check the user permission status to android Api>=23

DurankGts and others added 3 commits May 19, 2017 09:22
changes from central respository
180717_addingChangesFromCn1Repository
----------------------------------------
1-new method in LyfeCycleListener and AndroidNativeUtils to check the permission result
@codenameone
Copy link
Collaborator

My one concern with this is that it will break compilation for anyone who uses this interface which might mean cn1libs will break. Can we have an additional interface and check whether that is implemented using instanceof ?

@DurankGts
Copy link
Contributor Author

Ok. Please Do that

@codenameone
Copy link
Collaborator

I meant that you would update that

@DurankGts
Copy link
Contributor Author

I can't understand you. What do you want that I do?

@codenameone
Copy link
Collaborator

Right now if I merge this PR we will get a compilation error for anyone who used this feature before as you added a method to an existing interface.

You need to split that to a separate interface for compatibility.

@DurankGts
Copy link
Contributor Author

Can I set to default this method to avoid problem?

public interface LifecycleListener {
public void onCreate(Bundle savedInstanceState);
public void onResume();
public void onPause();
public void onDestroy();
public void onSaveInstanceState(Bundle b);
public void onLowMemory();
public default void onRequestPermissionsResult(int requestCode,boolean permissionStatus){};
}

@codenameone
Copy link
Collaborator

No. We can't use Java 8 in the repos yet. We would like to add that in the future but our last attempt to do that failed

@DurankGts
Copy link
Contributor Author

do you add Java 8 to your Repository in this pull request?

No. We can't use Java 8 in the repos yet. We would like to add that in the future but our last attempt to do that failed

@codenameone
Copy link
Collaborator

No. It will take a while to add that capability.

@DurankGts
Copy link
Contributor Author

When do have programmed to include Java 8 to your Repository in this pull request?

@codenameone
Copy link
Collaborator

We already tried to move to Java 8 and failed. It's a very risky task as we have a lot of legacy code around and this could break builds for everyone. Right now it's not a priority but that might change.

@DurankGts
Copy link
Contributor Author

When do have programmed to include Java 8 to your Repository in this pull request?

@codenameone
Copy link
Collaborator

It's hard to priorities this as it's not an urgent feature & carries a lot of risks/problems with it.

@DurankGts
Copy link
Contributor Author

When do have programmed to include Java 8 to your Repository in this pull request?

1 similar comment
@DurankGts
Copy link
Contributor Author

When do have programmed to include Java 8 to your Repository in this pull request?

@codenameone
Copy link
Collaborator

It won't be in 5.0

@DurankGts
Copy link
Contributor Author

When do have programmed to include Java 8 to your Repository in this pull request?

@codenameone
Copy link
Collaborator

We don't have it scheduled yet. I suggest revising the pull request to use Java 5 syntax

@DurankGts
Copy link
Contributor Author

When do have programmed to include Java 8 to your Repository in this pull request?

1 similar comment
@DurankGts
Copy link
Contributor Author

When do have programmed to include Java 8 to your Repository in this pull request?

@codenameone
Copy link
Collaborator

No. I don't understand the problem. Just remove the usage of lambdas and replace them with anonymous inner classes.

@DurankGts
Copy link
Contributor Author

I can't understand your solution. Please provide me an example. I created this pull request because I want create my custom dialog from the runtime native permission and you have a default dialog in your code that if very ugly. this solution if to listen the result of the user when the run time native permission if showed.

@codenameone
Copy link
Collaborator

You are adding a method to an interface. You can't do that without default methods. You will break compatibility with code that relies on that interface. You can create a separate interface e.g. RequestPermissionResult which implements only that method. Then:

    static void onRequestPermissionsResult(int requestCode,boolean permissionStatus) {
        if(listeners != null) {
            for(LifecycleListener l : listeners) {
                if(l instanceof RequestPermissionResult) {
                    ((RequestPermissionResult)l).onRequestPermissionsResult(requestCode,permissionStatus);
                }
            }
        }
    }

Your concrete listener can implement both interfaces whereas everyone else will implement only the original interface.

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

2 participants