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.

Sign up for GitHub

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

Jump to bottom

[Android] Can we allow views accept Context? (not ThemedReactContext) #5053

Closed
deminoth opened this issue Dec 31, 2015 · 6 comments
Closed

[Android] Can we allow views accept Context? (not ThemedReactContext) #5053

deminoth opened this issue Dec 31, 2015 · 6 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@deminoth
Copy link
Contributor

I've made a RN android app with the YouTube Android Player API. In the API, YouTubePlayerView constructor requires YouTubeBaseActivity as a context parameter. My native module is like below:

  @Override
  public YouTubePlayerView createViewInstance(ThemedReactContext context) {
    mContext = context;

    mYouTubePlayerView = new YouTubePlayerView(mActivity);
    mYouTubePlayerView.initialize(YOUTUBE_API_KEY, this);

    return mYouTubePlayerView;
  }

But when the RN drop this view, it notify view manager with onDropViewInstance,

resolveViewManager(view.getId()).onDropViewInstance(
  (ThemedReactContext) view.getContext(),
  view);

and it shows the redbox ...MainActivity cannot be cast to com.facebook.react.uimanager.ThemedReactContext.

I'm applying a workaround by checking view.getContext() instanceof ThemedReactContext. Is it only my problem? Or can we allow Context as a constructor parameter of views? Any other suggestions?
@facebook-github-bot
Copy link
Contributor

Hey deminoth, thanks for reporting this issue!

React Native, as you've probably heard, is getting really popular and truth is we're getting a bit overwhelmed by the activity surrounding it. There are just too many issues for us to manage properly.

  • If you don't know how to do something or something is not working as you expect but not sure it's a bug, please ask on StackOverflow with the tag react-native or for more real time interactions, ask on Discord in the #react-native channel.
  • If this is a feature request or a bug that you would like to be fixed, please report it on Product Pains. It has a ranking feature that lets us focus on the most important issues the community is experiencing.
  • We welcome clear issues and PRs that are ready for in-depth discussion. Please provide screenshots where appropriate and always mention the version of React Native you're using. Thank you for your contributions!

@brentvatne
Copy link
Collaborator

cc @kmagiera @foghina

@foghina
Copy link
Contributor

foghina commented Jan 4, 2016

You shouldn't store a reference to your activity, and instead pass the ThemedReactContext to the YouTubePlayerView constructor. Is there a specific reason why you're holding a reference to the activity and passing it instead?

@foghina
Copy link
Contributor

foghina commented Jan 4, 2016

Nevermind, just saw the constructor for YouTubePlayerView needs a special type activity. Hmm.

@foghina
Copy link
Contributor

foghina commented Jan 4, 2016

We can probably change the signature of onDropViewInstance to take a Context instead, and view managers can cast to themed if they really need to. However, most managers don't even implement this method AFAIK, so the signature change should be pretty safe. Would you mind sending a PR for this?

deminoth added a commit to dobbit/react-native that referenced this issue Jan 5, 2016
@deminoth
Change the signature of onDropViewInstance. issue facebook#5053
b824e08
@deminoth deminoth mentioned this issue Jan 5, 2016
Closed
@deminoth
Copy link
Contributor Author

deminoth commented Jan 5, 2016

@foghina #5125

ghost pushed a commit that referenced this issue Jan 6, 2016
@deminoth
Change the signature of onDropViewInstance. issue #5053
daa93a6 
Summary:
#5053
Closes #5125

Reviewed By: svcscm

Differential Revision: D2807202

Pulled By: foghina

fb-gh-sync-id: 1e268c940a08aa7bf243971ba91d4595973f12af
christopherdro pushed a commit to wildlifela/react-native that referenced this issue Jan 20, 2016
@deminoth @christopherdro
Change the signature of onDropViewInstance. issue facebook#5053
df60f37 
Summary:
facebook#5053
Closes facebook#5125

Reviewed By: svcscm

Differential Revision: D2807202

Pulled By: foghina

fb-gh-sync-id: 1e268c940a08aa7bf243971ba91d4595973f12af
@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 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@brentvatne @foghina @deminoth @facebook-github-bot @react-native-bot