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

[Android][0.29] No longer possible to access Activity from SimpleViewManager #8661

Closed
marcshilling opened this issue Jul 8, 2016 · 18 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@marcshilling
Copy link

According to the 0.29.0 upgrade docs, it's now very easy to extend ReactContextBaseJavaModule and use getCurrentActivity to get the activity reference in native Android modules. However, it no longer appears to be possible to get access to the current activity from native Android UI Components.

Continuing a discussion started here, I need access to the activity so that I can create a FragmentManager to add a Fragment to the FrameLayout being vended from SimpleViewManager. There was a suggestion that ReactContext.getCurrentActivity() be made public, which seems like a bad idea, but I can't think of another way.

@satya164
Copy link
Contributor

Have you tried 49f20f4#commitcomment-18177294 ?

@marcshilling
Copy link
Author

Yeah, that's me discussing the issue in those comments also 😛 . After that discussion went nowhere, I decided to open this issue.

@satya164
Copy link
Contributor

@marcshilling yes, but you didn't reply if view.getContext works.

@marcshilling
Copy link
Author

It didn't.

marcshilling referenced this issue Jul 13, 2016
Reviewed By: astreet

Differential Revision: D3328342

fbshipit-source-id: af4e825d0b7c2d3d4490094a939e97cc527dd242
@InnerPeace080
Copy link
Contributor

InnerPeace080 commented Jul 20, 2016

hi . this is my workaround to solve this problem . hope it will help u
InnerPeace080@c0838f0

now. we can use getPackages(Activity activity)

@marcshilling
Copy link
Author

@InnerPeace080 you should send a pull request so we can get this in core!

@nickjanssen
Copy link

+1 I also cannot access Activity any longer, I need it in the constructor phase in order to get a SplashScreen working. Calling getCurrentActivity() in the constructor simply returns null.

public class SplashScreen extends ReactContextBaseJavaModule {

    public SplashScreen(ReactApplicationContext reactContext) {
        super(reactContext);

        getCurrentActivity() // null
    }

@marcshilling
Copy link
Author

@nickjanssen that's actually different from the issue being discussed here. You are extending ReactContextBaseJavaModule, which has access to getCurrentActivity(). See this comment about why you can't use getCurrentActivity() in the constructor.

This issue is regarding SimpleViewManager extensions not having getCurrentActivity(), which is an issue that still remains in 0.30.0.

@skevy
Copy link
Contributor

skevy commented Jul 22, 2016

@foghina any thoughts on this? Seems like something super useful to get fixed.

@foghina
Copy link
Contributor

foghina commented Jul 22, 2016

Having getPackages take an Activity parameter is a much worse idea than making ReactContext#getCurrentActivity() public, for the following reasons:

  1. We don't want to tie package / module / viewmanager (and ultimately ReactContext / react instance) creation to an Activity, as we want to be able to initialize a react instance in any context.
  2. As the current in getCurrentActivity() suggests, the Activity can change over time. Holding a reference to whatever the activity was at react instance creation time is both incorrect and a memory leak.

As long as we properly document getCurrentActivity() (I think it already is documented in caps that you shouldn't hold references to the result), it should be fine to make it public.

ghost pushed a commit that referenced this issue Jul 29, 2016
Summary:
Addresses #8661
Closes #9071

Differential Revision: D3641285

Pulled By: foghina

fbshipit-source-id: dede86743efddc33b6ead053e805770fc213685c
grabbou pushed a commit that referenced this issue Aug 1, 2016
Summary:
Addresses #8661
Closes #9071

Differential Revision: D3641285

Pulled By: foghina

fbshipit-source-id: dede86743efddc33b6ead053e805770fc213685c
bartolkaruza pushed a commit to immidi/react-native that referenced this issue Aug 3, 2016
Summary:
Addresses facebook#8661
Closes facebook#9071

Differential Revision: D3641285

Pulled By: foghina

fbshipit-source-id: dede86743efddc33b6ead053e805770fc213685c
@marcshilling
Copy link
Author

This is fixed in v0.31.0 because you can now call getCurrentActivity() on ReactApplicationContext 👍

@lschmierer
Copy link

In SimpleViewManager#createViewInstance(), ThemedReactContext#getCurrentActivity() seems to return null.
This is probably because of the Activity is not initialized when ReactNativeHost is created in the Application.

@marcshilling
Copy link
Author

@lschmierer correct

@lschmierer
Copy link

So, what to do in this case? Is there any way to create the ViewManager later on?

@marcshilling
Copy link
Author

I'm not sure. What I'm doing is creating a placeholder object:

    @Override
    protected FrameLayout createViewInstance(ThemedReactContext context) {
        mFrameLayout = new FrameLayout(context);
        return mFrameLayout;
    }

And then actually doing the work of adding the activity-dependent view to the FrameLayout later on when props change.

@lschmierer
Copy link

Thank you, this seems to be a decent woraround!

@foghina
Copy link
Contributor

foghina commented Aug 9, 2016

In SimpleViewManager#createViewInstance(), ThemedReactContext#getCurrentActivity() seems to return null.

Hmm, that's a bug IMO. It happens because we call startApplication in onCreate() but we only set the current activity in onResume(). Created #9310 to track.

@lschmierer
Copy link

ThemedReactContext#getCurrentActivity() seems to be allways null in react-native version 0.31.0. At least on property setting it is still null.
@marcshilling on which react-native version did your workaround worked?

bubblesunyum pushed a commit to iodine/react-native that referenced this issue Aug 23, 2016
Summary:
Addresses facebook#8661
Closes facebook#9071

Differential Revision: D3641285

Pulled By: foghina

fbshipit-source-id: dede86743efddc33b6ead053e805770fc213685c
mpretty-cyro pushed a commit to HomePass/react-native that referenced this issue Aug 25, 2016
Summary:
Addresses facebook#8661
Closes facebook#9071

Differential Revision: D3641285

Pulled By: foghina

fbshipit-source-id: dede86743efddc33b6ead053e805770fc213685c
@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

9 participants