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

React Native Web support #1664

Merged
merged 9 commits into from
Jan 17, 2019

Conversation

haruelrovix
Copy link
Contributor

@haruelrovix haruelrovix commented Dec 13, 2018

Implements #1423

  • Write docs for React Native Web support
  • Create a compelling demo/example app for RNE + RNW

The example app LIVE here: gitphone.surge.sh.
For the source code: haruelrovix/gitphone.

I think it's better if the example source moved under react-native-training. Also, I'm planning to publish the RNE x RNW documentation on Medium as well. So it can reach more audience. Draft

Create a React Native Web support section on the main Readme.md
using React Native Web and React Native Elements
@codecov
Copy link

codecov bot commented Dec 13, 2018

Codecov Report

Merging #1664 into next will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             next    #1664   +/-   ##
=======================================
  Coverage   87.74%   87.74%           
=======================================
  Files          33       33           
  Lines         555      555           
  Branches       68       68           
=======================================
  Hits          487      487           
  Misses         57       57           
  Partials       11       11

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 940c407...732b761. Read the comment docs.

@iRoachie
Copy link
Collaborator

YES YES YES!!!! :D I am so excited

@Monte9
Copy link
Collaborator

Monte9 commented Dec 13, 2018

OMG!! This is what we have been waiting for. I ❤️ it!! And the app looks really clean too! 😍

Also this PR is awesome in every way!! It's got an in-detail readme guide, a blog post, an awesome example app. 💯 🔥

Great job @haruelrovix and thanks again for contributing! I'll review it in a day or so and then let's get this merged in!

@xavier-villelegier
Copy link
Collaborator

This is amazing !! Thanks for your help on this @haruelrovix. Would be dope to write "Compatible with React Native Web" on the home page 🔥

We have an old branch trying to make RNE compatible with RNW but there was a lot of changes necessary, so we didn't want to merge it into master. Looking at your app (really clean btw !!) it seems to work out of the box ? Did you find some components of RNE that doesn't work with RNW ? If so, is it easy to make them compatible ? 🤔

Looking forward to merge it ! 💯

@xavier-villelegier
Copy link
Collaborator

Actually I was talking about this big PR https://github.com/react-native-training/react-native-elements/pull/759

@iRoachie
Copy link
Collaborator

I wouldn't really compare the two cause they aren't the same. That PR also added storybook as well as added web specific tests for each component and their snapshots.

This Pr is more a "how to get started with react-native-web and react-native-elements"

@xavier-villelegier
Copy link
Collaborator

Yea but looking at the PR there was many changes to the components to make them compatible with RNW, that's why I ask if @haruelrovix found some incompatible components

@haruelrovix
Copy link
Contributor Author

haruelrovix commented Dec 15, 2018

@xavier-villelegier after did extra setup on the webpack configuration: 8c0e603 4fa1a4a, I can say that RNE works out of the box with RNW!

Had to use platform specific style in ListItem, L22

marginRight: Platform.OS === 'web' ? 10 : 0

to achieve the same feel on native and web. But it's really acceptable.

Last year I was in RNW project. We used platform specific style for web, android, iOS and iPhone X. Yes. We had to give iPhone X a special styling. 😆

This simple gitphone app uses 7 RNE components only, out of 20. Don't know the other RNE components, work out of the box with RNW or not.

@andangrd
Copy link

@haruelrovix Awesome..... 👏 👏 👏

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@@ -0,0 +1,618 @@
# Comprehensive Guide to create simple app using React Native Web and React Native Elements
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is one of the most detailed and step by step instructions I have seen. Fantastic job on this! 👏 💯


For native, it’s easier. We can use `FlatList` `onEndReached` props. To give you an idea how, see this: [9d2e1f2](https://github.com/haruelrovix/rnw-github/pull/2/commits/9d2e1f2a3e8605f8184f1f8950eb0175045bb67a).

## Troubleshooting 💺
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow. you even took the time to write a troubleshooting section. Impressive!! 🔥

Copy link
Collaborator

@Monte9 Monte9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic!! 💯

@Monte9
Copy link
Collaborator

Monte9 commented Jan 13, 2019

@haruelrovix left a few comments. But overall really impressed by your efforts on this front and investing time & effort into making a really compelling RNE x RNW app. ❤️

We use OpenCollective to give back to contributors like yourself. Our initial budget for RNW docs for $50.. but seeing that you have gone above and beyond to make a fully functioning app is really impressive. So I'd be happy to send $100 your way for your efforts. Let me know if this sounds good to you and I can tell you how to submit your expense.

@Monte9
Copy link
Collaborator

Monte9 commented Jan 13, 2019

@iRoachie also looks like @piu130 submitted a few PR to help improve RNW support as well. How about we award him $50 on OpenCollective as well for his efforts.

@haruelrovix
Copy link
Contributor Author

Thanks for the review @Monte9 ! 🤝

Related to the OpenCollective, isn't it $100 from the very beginning? 🤔

  • Write docs for React Native Web support $50
  • Create a compelling demo/example app for RNE + RNW $50

@iRoachie
Copy link
Collaborator

Can we update the copy on the home page?

https://github.com/react-native-training/react-native-elements/blob/next/website/pages/en/index.js#L109. Maybe it should say and web and a link to the page you added above.

@haruelrovix
Copy link
Contributor Author

@iRoachie like this?

content: 'Consistent design across android, iOS and [web!](https://github.com/react-native-training/react-native-elements/tree/master/docs/rnw_support.md)'

@iRoachie
Copy link
Collaborator

Hey @haruelrovix, I updated the PR to host your setup as a blog post instead. If you want to preview it you can run the website locally.

# after clone
cd website
npm install
npm start

You can then click the blog tab at the top.

Let me know if everything is fine, here's a quick screenshot.

image

@Monte9
Copy link
Collaborator

Monte9 commented Jan 17, 2019

woah, we can have blog posts on the website?! 🔥 Docusaurus is really fantastic! 💯

@haruelrovix
Copy link
Contributor Author

@iRoachie oh man~ why are you so cool? Thanks for the last two commits. It's awesomesauce! 🎆

@iRoachie
Copy link
Collaborator

Ha! I think we are a go 🚢

@iRoachie iRoachie merged commit ee18171 into react-native-elements:next Jan 17, 2019
@iRoachie
Copy link
Collaborator

Thanks so much 💯

@xavier-villelegier
Copy link
Collaborator

@haruelrovix I just approved your expense on OpenCollective, thanks again man, what an awesome PR ! 🥇 I'm sure it'll be very helpful for everyone.

The blog posts are awesome, so many possibilities ! Thanks for letting us know that it exists @iRoachie

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants