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

Update showcase.js #8394

Closed
wants to merge 1 commit into from
Closed

Conversation

alexHlebnikov
Copy link

Thanks for submitting a pull request! Please provide enough information so that others can review your pull request:

(You can skip this if you're fixing a typo or adding an app to the Showcase.)

Explain the motivation for making this change. What existing problem does the pull request solve?

Prefer small pull requests. These are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it.

Test plan (required)

Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI.

Make sure tests pass on both Travis and Circle CI.

Code formatting

Look around. Match the style of the rest of the codebase. See also the simple style guide.

For more info, see the "Pull Requests" section of our "Contributing" guidelines.

@ghost
Copy link

ghost commented Jun 24, 2016

By analyzing the blame information on this pull request, we identified @lacker and @brentvatne to be potential reviewers.

@ghost
Copy link

ghost commented Jun 24, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@hramos
Copy link
Contributor

hramos commented Jun 24, 2016

Hey @alexHlebnikov, we are changing the way the showcase works so that everything now has a link to some external content, like press coverage by a well-known publisher, or a deep technical blog post detailing the development process using React Native. Unfortunately it does not seem like this application meets these requirements at this time.

@hramos hramos closed this Jun 24, 2016
@alexHlebnikov
Copy link
Author

alexHlebnikov commented Jun 24, 2016

Hi @hramos !

Can you tell me more details which requirements were doesn't met?
Because Sib.fm is a well known in Siberia internet media.
Also, the app is branded with a public company brand - 2GIS.

Why these two conditions are not enough?

@lacker
Copy link
Contributor

lacker commented Jun 24, 2016

Well, the link should just be useful for the average reader of the website. Usually that means, there's some reason it would get clicked on, and then there's some useful content at the link. In particular it probably has to be in English to be useful to the readers of the website - the language is vast majority English, a little bit Chinese, and then a huge dropoff to #3. Even the Chinese apps we are including here have English-language links. The only stuff I can find looks Russian, so, it just doesn't seem like it would be something we can get people to click on.

@alexHlebnikov
Copy link
Author

Thank you for answer, @lacker.

react-one pushed a commit to react-one/react-native that referenced this pull request Sep 24, 2021
In the current implementation the ref is unused, resulting in a constant `current: {null}` on the context.
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