Skip to content

Update project #6

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

Merged
merged 2 commits into from
Sep 20, 2018
Merged

Update project #6

merged 2 commits into from
Sep 20, 2018

Conversation

Freddy03h
Copy link
Contributor

Hello, thank's for this example repo !
I tested it and I want to update dependencies.

But I'm a newbie in Reason, so I'll try to explain the changes I made and I need reviews please.

bs-react-native related

Add String() to color and backgroundColor styles.

reason-react related

I run the script to update ReasonReact from 0.4.2-to-0.5.0
https://github.com/chenglou/upgrade-reason-react
It updated Style.( to { open Style;, and ReactEventRe.Mouse to ReactEvent.Mouse and some other syntax changes (remove some brackets, add some commas, etc)

Use the new Subscriptions helper in ClientRoot.

react-native-web related

Froze react and react-dom to 16.4.X because it has breaking changes with 16.5.X

Change global CSS and server style SSR from react-native-web doc.

Add react-art : it's noted as optional on react-native-web getting started documentation, but I don't find a way to do without it by default.

Create a <TextLink> component using <Text> from react-native-web instead of an a element. It's in JS because bs-react-native don't take into account specific props only available in react-native-web.
I took exemple from this code : https://github.com/MoOx/moox.io/blob/2b7d26596e9da068b9fa95fe0f4190db8908a0be/src/components/TextLinkNative.js
And I ended up with 3 files, I don't know if it's ok or not.

Thank's

@Freddy03h
Copy link
Contributor Author

Freddy03h commented Sep 20, 2018

react-native-web 0.9 is now compatible with react(-dom) 16.5 since version 16.5.1

@kgoggin
Copy link
Collaborator

kgoggin commented Sep 20, 2018

Thanks @Freddy03h for the updates! The TextLink component is interesting - I hadn't seen that approach before as a way to remove any reference to a Dom element in the code. I've also run into the problem with bs-react-native not accepting the web-specific props and found that frustrating. FWIW, the approach I'm taking in the RNW app I'm building is to have a web-specific component that renders an <a/> so that it's semantically correct, and then I just make sure not to pull that component in to the native code.

Anyway, thanks for the PR!

@kgoggin kgoggin merged commit 77f5373 into jaredpalmer:master Sep 20, 2018
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.

2 participants