search

status-im / status-components

A set of React/React native components used by Status.im projects
https://status-im.github.io/status-components
7 stars 9 forks source link

Add support for react-native #3

Closed jeluard closed 5 years ago

jeluard commented 5 years ago

Setup storybook so that it supports react-native.

As much as possible it would be great to have everything in a single source tree. Even sharing code if that's feasible. Something like https://github.com/necolas/react-native-web could be useful? Components would be react-native first then. Also see:

gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 300.0 DAI (300.0 USD @ $1.0/DAI) attached to it as part of the Status.im fund.

jeluard commented 5 years ago

@eswarasai Are you still interested in this one?

eswarasai commented 5 years ago

@jeluard -- Looks like someone had already applied for this. I'll let @grigio take a shot at this while I get the color palette story done :)

jeluard commented 5 years ago

@eswarasai Sorry about that!

grigio commented 5 years ago

@jeluard Hi, I did some tests and it seems storybook 5 is still incompatible with storybook 5.x

A possible solution could be to downgrade the project to storybook 4.x or wait until storybook 5.x reenable the RN support source: https://github.com/storybooks/storybook/issues/5893

jeluard commented 5 years ago

SGTM!

Can you detail what you plan to do (based on the issue details)?

grigio commented 5 years ago

I can't estimate when it will be fixed in mainstream.So I'd try to downgrade your project to 4.x and follow https://gist.github.com/johnwylie70/1f019c643641f56b400acc78415d398d as specified in the issue. Is OK for you?

jeluard commented 5 years ago

@grigio I mean how do you plan to support both react and react-native components? See this issue description for examples approaches.

I'd rather have you detail your plan before you invest time in it, just to make sure we are on the same page.

grigio commented 5 years ago

@jeluard Ah ok, because in Gitcoin the issue hasn't been updated. If you are referring to react-native-web it is an abstraction and some components couldn't be available to the web. My proposal to complete this issue is to have a working react-native version and check some basic react-native-web compatibility (bc you wrote you are interested in a React-Native first project)

jeluard commented 5 years ago

The goal of the issue is to have a strategy and the repo ready so that components supported on both react-native and react can be created. React support is already setup. By react-native first I mean the syntax used to declare the components (probably react-native like and use react-native-web if needed for web support).

Sorry if that wasn't clear enough.

grigio commented 5 years ago

Sure, If the project has RN style components it should also work with RNW. I just wanted to specify that some features could need a different implementation from platform to platform. (es. in-app purchases)

jeluard commented 5 years ago

Perfect! Just wanted to make sure that we are on the same page! :D Ideally you could create a dumb button or something simplistic to demo the usage.

Most of our components are pretty low level so I feel like we can achieve high reuse>

grigio commented 5 years ago

https://github.com/grigio/status-components-sandbox

What have I done?

  1. Investigated the current situation. Storybook 5 do not play with React Native
  2. There isn't an official way to have a Storybook that support React and React Native
  3. Followed the suggested links to find a proper solution

What am I delivering?

  1. Boilerplate based on Kiwi/Margarita that supports Storybook 4 both React Native on Web and on Mobile (tested with Expo Android)
  2. A dummy Button component to show the usage
gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 300.0 DAI (300.0 USD @ $1.0/DAI) has been submitted by:

  1. @grigio

@StatusSceptre please take a look at the submitted work:

jeluard commented 5 years ago

Is that a PR on top of this repository?

Also please describe the technical solution you chose and why it's the right one from the different example I shared.

grigio commented 5 years ago

Is that a PR on top of this repository? No

Also please describe the technical solution you chose and why it's the right one from the different example I shared.

This https://github.com/status-im/status-components/issues/3#issuecomment-473814654 I tried all the links you suggested and others I considered. I think this is the right one because:

jeluard commented 5 years ago

@grigio Unless I am missing something, this is not a PR but a fork of a repo. It's not usable for us as is.

What I am expecting is one commit on top of this repo that address this issue with minimal changes.

Have you considered microsoft approach? At first glance it looks very well maintained.

grigio commented 5 years ago

@jeluard Yes it's a fork, I also thought to "just" add Storybook to your project but currently to have a working combo RN (web+native) other behind the scene efforts are required.

The best approch would be https://github.com/khronedev/react-native-hybrid-storybook but it doesn't work anymore because it requires even older deps

It doesn't have a Storybook support and MS doesn't care about it https://github.com/Microsoft/reactxp/issues/255#issuecomment-328003205

jeluard commented 5 years ago

other behind the scene efforts are required.

Do you have more details about that?

Most important is to have a sound approach to create shared components for both reat/react-native, storybook support for react-native can come later on.

grigio commented 5 years ago

The main missing component is @kiwicom/margarita-mobile-storybook which also depends on other kiwicom internal projects.

If it isn't a priority React Native now, the simplest approch is to use react-native-web (not React) and it officially support Storybook 3.x https://github.com/necolas/react-native-web/blob/master/packages/website/package.json#L12

grigio commented 5 years ago

Just a prove that the code below supports RNW on Web and on Android

image

jeluard commented 5 years ago

Yes react-native-web sounds great! I'd rather wait for RN support, I saw somewhere something is cooking with regards to latest storybook.

grigio commented 5 years ago

If you use react-native-web in the way I did the react-native support should be very easy when possible https://github.com/status-im/status-components/pull/9

jeluard commented 5 years ago

Thanks! What do you mean by react-native support should be easy?

Some more links that can be useful:

grigio commented 5 years ago

I mean if the components will be made just with RNW apis https://github.com/necolas/react-native-web#components it's OK, but if you use also browser specific features your RNW components will work on the Web Browser but will need to be adapter to also work in RN

jeluard commented 5 years ago

Looking at https://github.com/necolas/react-native-web#examples I see they are using react-native API. WDYT? Can we do that? I'd rather not depend on `react-native-web API.

Also we can forget specific web features.

grigio commented 5 years ago

I've enable the alias, now you can do import { Button as Btn } from 'react-native'; even if you are using rnw behind the scene

jeluard commented 5 years ago

Nice thanks! Have you seen my other comment in your PR?

grigio commented 5 years ago

@jeluard can you summarize what is missing to merge this changes?

jeluard commented 5 years ago

Some things I would consider for a complete PR:

Also I would like to validate the result so probably will require #5 to be fully available.

This PR is a bit specific as it is foundation work. It's more about making sure we are ready to create new components, required changes are not very technical.

Is there some medium we could use to chat? I am available in status #status-core channel.

grigio commented 5 years ago

OK I've implemented them here https://github.com/status-im/status-components/pull/9#commits-pushed-e41893f

I consider the task complete. The #5 is not related, because the code works and also the tests

jeluard commented 5 years ago

@grigio Can you squash your commits?

grigio commented 5 years ago

@jeluard Done here https://github.com/status-im/status-components/pull/9/commits/3b579003fc731f6738b86a28c800019f55e7860e

jeluard commented 5 years ago

Fixed by #9

StatusSceptre commented 5 years ago

Our end shows that you were paid out. https://etherscan.io/tx/0xa9e55e44cdfdccd6c6f5cba0fdd87c32c35334dfb0a460354706cf60e139d493

grigio commented 5 years ago

Yes received, thanks