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

☂️ Revamp RNTester #24647

Closed
5 tasks
pvinis opened this issue Apr 29, 2019 · 47 comments
Closed
5 tasks

☂️ Revamp RNTester #24647

pvinis opened this issue Apr 29, 2019 · 47 comments
Labels
Bug Help Wanted :octocat: Issues ideal for external contributors. Ran Commands One of our bots successfully processed a command.

Comments

@pvinis
Copy link
Contributor

pvinis commented Apr 29, 2019

Introduction

RNTester is the easiest way to demostrate what React Native can do. It has been that for a long time, but for a while now it hasn’t received much love. It has been in a kind of maintenance mode. It’s time to make RNTester shine!

Tasks

  • Make sure it works with the current latest release (0.59.5)
    This is just the base of everything.
  • Make sure it always is up to date with any new release
    We can use RNTester as a testbed of the upgrading process. It is also important for people to be able to clone the react-native repo and compile and run RNTester using the latest release.
  • Use a navigation library
    I guess this is self-explanatory. React Native does not provide navigation, so this will make a great example of one, and at the same time we can simplify the current navigation in RNTester.
  • Simplify the way we have the list of examples
    The RNTesterList files can probably be simplified, maybe we can put the key in the actual example. Ideally this list file would be one line per example, not 4-5 like now.
  • Have CI build RNTester and distribute to appstore/playstore
    This will be a good way to show off any new features/UI, and make it easy for someone to just see the app in action, without needing to setup and compile themselves.

Ideas for discussion

  • Extract to a separate repo?
    Would that make is easier to handle/upgrade?
  • Make sure it always is up to date with latest master?
    Is it useful to have RNTester up to date with master, so new development there can use RNTester as a testbed and showcase of new functionality/features/UI?
  • Use react-native-navigation or react-navigation?
    Fight!

Appendix

Inspiration issue

@cpojer

@react-native-bot

This comment has been minimized.

@react-native-bot react-native-bot added Ran Commands One of our bots successfully processed a command. Resolution: Needs More Information labels Apr 29, 2019
@pvinis pvinis changed the title Revamp RNTester ☂️ Revamp RNTester Apr 29, 2019
@cpojer
Copy link
Contributor

cpojer commented Apr 29, 2019

Thanks so much for starting this issue! I hope people will sign up to help us with this revamp effort to give RNTester a fresh new look and so that we can be proud of our example app :)

One key thing to me is to apply the styling that we are applying to the new app screen so that we make it look super fresh, see react-native-community/discussions-and-proposals#122. For styling, let's wait until we merge the new app screen and then ask somebody to help us out with restyling RNTester.

@pvinis
Copy link
Contributor Author

pvinis commented Apr 29, 2019

I'm willing to work on the CI stuff, but of course any help is appreciated for that and any other task! :)

@chakrihacker
Copy link
Contributor

I would like to help

@AmFlint
Copy link

AmFlint commented Apr 29, 2019

Hi ☺️
I've been working a lot with React native and expo recently.
I came to experience many different use cases for navigation. I used react-navigation and it worked pretty well after some time getting accustomed to it.
I am willing to help on this RNtester example for navigation or something else if needed :)

@brentvatne
Copy link
Collaborator

brentvatne commented Apr 29, 2019

I don't have a strong opinion on the navigation library to be used here, it probably doesn't matter too much which one we go with. I can help if it uses React Navigation though.

edit: one small argument in favour of React Navigation is that it may be less likely to break between releases because the JS-facing API of React Native is generally a lot more stable than the native side, and usually version changes don't impact React Navigation at all.

@ericlewis
Copy link
Contributor

@brentvatne another argument in favor of React Navigation is simplicity as well, having spent time in React Native Navigation for instance, it is not exactly "newb" friendly.

@RichardLindhout
Copy link

Maybe show examples with React Native Paper since most of the components are extracted from core.

Like Flutter does in their example app. Giving it a really customized look so people can see almost anything is possible in React Native.

@RichardLindhout
Copy link

Maybe even a web version with React Native web to show the cross platform possibilities!

@gvarandas
Copy link
Contributor

I have just upgraded our app to 0.59 and can definitely work towards that on RNTester.

@dulmandakh
Copy link
Contributor

I think RNTester must target master, because it's much easier to test changes I make, but also make sure that it didn't break anything.

@cpojer
Copy link
Contributor

cpojer commented Apr 30, 2019

Thanks everyone for all your enthusiasm on this. RNTester primarily exists to visualize all components that are part of React Native, ideally making them easy to test manually but also enable automated tested in some form. Including third-party libraries like navigation to showcase how to build React Native apps seems like a separate purpose that is best served through a separate app. We have discussed building a "community RNTester" outside of the main repo that includes the top third-party modules to ensure we keep these modules working in new releases. I think that app is much closer to the idea of an example app, and using a third-party navigation solution is a better fit there.

For RNTester itself, I would prefer it to have no dependencies except for what is part of the repo already.

@kelset kelset added Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. and removed Resolution: Needs More Information labels May 3, 2019
@duggiemitchell
Copy link

I'd like to help too 😊

@midastouchprd
Copy link

I'd love to get into this as well

@cpojer
Copy link
Contributor

cpojer commented May 8, 2019

If somebody wants to start sending Pull Requests to simply move files around so they are more organized, that would already be super helpful. Right now all the JS files are in a single folder and it is really hard to navigate.

@gvarandas
Copy link
Contributor

@cpojer Right on.

@pvinis
Copy link
Contributor Author

pvinis commented May 8, 2019

I need to also start helping. I'll have some time in the weekend.

@jeanregisser
Copy link
Contributor

Make sure it always is up to date with latest master?
Is it useful to have RNTester up to date with master, so new development there can use RNTester as a testbed and showcase of new functionality/features/UI?

I'm a bit confused about this.

RNTester uses master and is built and tested (unit and integration) on every new commit automatically with Circle CI. Though tests are only targeting iOS AFAIK.

In the past, I have used it as a testbed when adding a new feature or tweaking something.

I haven't followed RN development too closely recently, has this changed?

@ericlewis
Copy link
Contributor

@jeanregisser targets android too. Also includes e2e interaction tests for both platforms.

That said; to OP, idk if I love the if splitting RNTester completely out. Maybe if we just cleaned it up, and adopted more of a monorepo feel, it could live there.

@Johan-dutoit
Copy link
Contributor

@cpojer I've submitted a PR that does the first round of moving things around. I definitely felt the pain of 'hard to navigate'. Should be somewhat simpler now to find the entry point and the examples.

@Johan-dutoit
Copy link
Contributor

@AndreiCalazans No worries mate. It looks like you've progressed more than I have. I was planning on doing the rest in the next pass (just wanted to get the ball rolling at least).

I'm happy to bin mine (once yours has been merged in) and continue with that.

@pvinis
Copy link
Contributor Author

pvinis commented May 24, 2019

I'm back on Monday and I will review. very cool! 💪

@kdawgwilk
Copy link

I would actually vote that react-native-navigation should be used because it is the most likely to break with new react-native releases. They use some APIs that are not common for other libraries and due to this react-native has broken their lib multiple times with API renames that the react-native team didn't think anyone was using. This has been the biggest pain point for us when upgrading to new versions of react-native

@pvinis
Copy link
Contributor Author

pvinis commented May 24, 2019

Should or should not? If it's more likely to break, that should be a reason to not use it, right?

@AndreiCalazans
Copy link
Contributor

I think he is suggesting to use it, to try to protect it from future changes.

@pvinis
Copy link
Contributor Author

pvinis commented May 25, 2019

Ah ok. Maybe we could have a comparison?

facebook-github-bot pushed a commit that referenced this issue May 28, 2019
Summary:
Changes RNTester, first attempt in the direction of improving the RNTester overall. Related ticket: #24647

Changed the `js` directory of the RNTester to have the following structure:
```
- js
    - assets
    - components
    - examples
    - types
    - utils
```
* **assets**
_Any images, gifs, and media content_

* **components**
_All shared components_

* **examples**
_Example View/Components to be rendered by the App_

 * **types**
_Shared flow types_

 * **utils**
_Shared utilities_

## Changelog

[General] [Changed] - Update folder structure of RNTester's JS directory.
Pull Request resolved: #25013

Differential Revision: D15515773

Pulled By: cpojer

fbshipit-source-id: 0e4b6386127f338dca0ffe8c237073be53a9e221
@shirakaba
Copy link

[kdawgwilk] I would actually vote that react-native-navigation should be used because it is the most likely to break with new react-native releases.

[cpojer] For RNTester itself, I would prefer it to have no dependencies except for what is part of the repo already.

I agree with cpojer here. I'm building a custom React renderer and am in fact porting the RNTester suite as a representative test set for my renderer; I love how the existing architecture of RNTester solves navigation without bringing in any external libraries (nor even depending on platform-specific navigation functionalities), as it's one less SDK for me to learn and one less library to port.

@RichardLindhout
Copy link

Maybe we should have the RNTester application with all basic react native things and another one with a lot of third party features to show what's possible. Flutter also did a very nice preview of their features in the gallery app, I think that we should show a lot more of the possibilities with React Native.

@pvinis
Copy link
Contributor Author

pvinis commented Jun 8, 2019

I think that's a good idea. Let's do this after the revamp is kinda done, and we can copy it and just add a bunch of react-native-community stuff or other libs.
Starting next week I can finally spend some time on RNTester.

@jeremyscatigna
Copy link

Is there a way I could help ? :)

@pvinis
Copy link
Contributor Author

pvinis commented Jul 8, 2019

Hey @jeremyscatigna! I am sorry I haven't picked this up sooner.

There is actually. You could pick any of these and make a PR with some basic changes and we can discuss more about any choices we wanna make. For example making it work with the latest RN version is independent. I guess the other things need more attention from me. I will be getting into that after 2 weeks, when I have a bunch of irl stuff finished.

Feel free to try the upgrade and/or ping me again. Also, if you have ideas for a better list structure or file structure, let's discuss :)

@satheeshwaran
Copy link

I am a mobile developer recently getting my hands dirty with React native and it is simply awesome. I would like to help. Please tell me what to do!

@pvinis
Copy link
Contributor Author

pvinis commented Jul 15, 2019

This particular issue might not be the friendliest for a newcomer. Maybe try one of the other issues?

@kelset could you actually remove the Good First Issue from here, until we have it set up for people to help, please? :)

@satheeshwaran
Copy link

@pvinis I can try!

@kelset kelset removed the Good first issue Interested in collaborating? Take a stab at fixing one of these issues. label Jul 15, 2019
@satheeshwaran
Copy link

@pvinis @kelset Can I work on a simple warning like this?
Simulator Screen Shot - iPhone X - 2019-07-16 at 12 21 03
i.e. Instead of using Slider from react-native I will use Slider from react-native-community?

@shirakaba
Copy link

shirakaba commented Jul 16, 2019

Hi @satheeshwaran, good to see your enthusiasm to contribute. Now I may be mistaken but that looks like a warning about your own app’s source code importing Slider from the core (which is deprecated) rather than from its newer community module. There’s therefore no relevant issue to be fixed in the core to address this.

I believe this is getting off-topic from the purpose of the RNTester discussion, however. I would suggest looking through the list of issues marked “good first issue”, and assessing whether any of those look manageable for you. Upon finding a suitable issue, I’d encourage raising discussion there.

@elicwhite
Copy link
Member

@shirakaba, I believe the error @satheeshwaran is pointing at is a yellow box that appears in RNTester because it includes tests for Slider. I believe that instead of changing RNTester to pull in the community module we are more likely to just remove the slider tests from the core RNTester.

@satheeshwaran
Copy link

@shirakaba Yes @TheSavior is right, this is the warning in the RNTester app(the screenshots says RNTesterApp.ios.bundle right?). There are a huge set of warnings in the RNTester app, can I start to work on fixing them. It should be easy for me to start actually :-)

@shirakaba
Copy link

I see now, my mistake! Thanks for clearing that up @TheSavior.

That raises another question, then. Should RNTester really have a dependency on community modules? I think it would be better for it to simply stop showcasing any non-core components. My reasoning follows @cpojer's thoughts from before:

For RNTester itself, I would prefer it to have no dependencies except for what is part of the repo already.

@satheeshwaran
Copy link

@shirakaba Ok I have started to work on removing non-core component examples, will get back soon.

@satheeshwaran
Copy link

I made the changes, please review and merge - #25746

@henrymoulton
Copy link
Contributor

Would like to get involved - have started the process by trying to get the test_ios_e2e workflow green in #26111

@henrymoulton
Copy link
Contributor

henrymoulton commented Aug 19, 2019

It'd be good to update the list of tasks with removal of tasks that are no longer desired:

In particular:

Introduction

RNTester is the easiest way to demonstrate what React Native can do. It has been that for a long time, but for a while now it hasn’t received much love. It has been in a kind of maintenance mode. It’s time to make RNTester shine!

Tasks

  • Use a navigation library
    I guess this is self-explanatory. React Native does not provide navigation, so this will make a great example of one, and at the same time we can simplify the current navigation in RNTester.

Ideas for discussion

  • Use react-native-navigation or react-navigation?

@cpojer has mentioned:

For RNTester itself, I would prefer it to have no dependencies except for what is part of the repo already.

I imagine this removes this task from the list?

Commenters have mentioned having a React Native project that new users can demo/view that demonstrates:

Perhaps the React Native Community organisation would be a good fit for this?

This could then be done in conjunction with:

Have CI build RNTester and distribute to App Store/Play Store. This will be a good way to show off any new features/UI, and make it easy for someone to just see > the app in action, without needing to setup and compile themselves.

@cpojer
Copy link
Contributor

cpojer commented Jan 24, 2020

Since we didn't have a lot of time to make RNTester better, what if we started simple: Could we update the design to look as pretty as the new app screen in terms of colors, fonts, spacing and all that? We can keep all the functionality the same and just iterate on the design and I think it could quickly improve the perception of RNTester. Anybody wanna take a stab at this?

@gvarandas
Copy link
Contributor

I could propose some changes.
If anyone with a design background wants to contribute as well, we can co-author a PR.

M-i-k-e-l pushed a commit to M-i-k-e-l/react-native that referenced this issue Mar 10, 2020
Summary:
Changes RNTester, first attempt in the direction of improving the RNTester overall. Related ticket: facebook#24647

Changed the `js` directory of the RNTester to have the following structure:
```
- js
    - assets
    - components
    - examples
    - types
    - utils
```
* **assets**
_Any images, gifs, and media content_

* **components**
_All shared components_

* **examples**
_Example View/Components to be rendered by the App_

 * **types**
_Shared flow types_

 * **utils**
_Shared utilities_

## Changelog

[General] [Changed] - Update folder structure of RNTester's JS directory.
Pull Request resolved: facebook#25013

Differential Revision: D15515773

Pulled By: cpojer

fbshipit-source-id: 0e4b6386127f338dca0ffe8c237073be53a9e221
@cortinico
Copy link
Contributor

Closing as this is not an effort we're currently looking into. PRs that improve RN Tester are always more than welcome 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Help Wanted :octocat: Issues ideal for external contributors. Ran Commands One of our bots successfully processed a command.
Projects
None yet
Development

No branches or pull requests