Examples basic form #59
Conversation
lloydwheeler
added
some commits
May 1, 2017
jongold
self-requested a review
May 1, 2017
jongold
requested changes
May 1, 2017
View changes
This is awesome, thanks so much for submitting it!
There's some duplication that I'd love to see removed (to fully showcase the power of primitives), but it's super close
If you're still unclear on architecture let me know.
I mentioned that flow Props were missing in a few components - if you'd prefer to use prop-types that's totally cool, but just make sure that every component has propTypes
Also, please make sure that eslint passes when you're ready to merge :)
| + "react-test-renderer": "^15.4.1" | ||
| + }, | ||
| + "devDependencies": { | ||
| + "skpm": "^0.8.0" |
jongold
May 1, 2017
Member
Can you update to ^0.9.0? I've been meaning to do this to the rest of them
| +} | ||
| + | ||
| + | ||
| +const Button = ({ label, backgroundColor }: Props) => ( |
| +import React from 'react'; | ||
| +import { View } from 'react-primitives'; | ||
| + | ||
| +const Register = ({ children }: Props) => ( |
jongold
May 1, 2017
Member
I think this could be called something more generic - it's not a 'registration component' per se, it's more like a 'column wrapper' — <Columns>?
| + return 'strong'; | ||
| +} | ||
| + | ||
| +const StrengthMeter = ({ password }: Props) => ( |
| @@ -0,0 +1,19 @@ | ||
| +import React from 'react'; |
jongold
May 1, 2017
Member
The main hurdle was (as far as I'm aware) there not being a react-primitives textbox so that's the reasoning behind having a TextBox component that has primitive and web components inside it, with shared styles across the two.
You don't have to explicitly import primitive.js & web.js —
- rename
TextBox/primitive.jstoTextBox/index.sketch.js - rename
TextBox/web.jstoTextBox/index.js
Then, import TextBox from './components/TextBox' will just work - Sketch will find the .sketch.js file, web will find the .web.js file
| +import styles from './style'; | ||
| +import StrengthMeter from '../StrengthMeter'; | ||
| + | ||
| +const TextBox = ({ label, value, type }: Props) => ( |
| + <View style={styles.formElement}> | ||
| + <Text style={styles.label}>{label}</Text> | ||
| + <View style={styles.textbox}>{value}</View> | ||
| + {type === "password" && value.length > 0 && |
jongold
May 1, 2017
Member
You could also render StrengthMeter directly in your form - it doesn't need to live inside TextBox explicitly —
~=
<View>
<TextBox />
<TextBox />
<StrengthMeter />
<Button />
</View>Alternatively, if you wanted it to be attached to the TextBox,
<View>
<TextBox />
<TextBox>
<StrengthMeter />
</TextBox>
<Button />
</View>& then in TextBox,
{ children && props.children }
| + <Register> | ||
| + {sessions.map(session => ( | ||
| + <Space key={session.password} h={spacing.Large} v={spacing.Large}> | ||
| + <View style={styles.register}> |
jongold
May 1, 2017
Member
I'd extract this whole thing to a new component ~= <RegistrationForm> that takes an email & password - then you could reuse it in the web version too
| + <Register> | ||
| + <div style={styles.register}> | ||
| + <Text style={{...styles.heading}}>Register an Account</Text> | ||
| + <TextBox |
jongold
May 1, 2017
Member
See my comment on src/main - you shouldn't have to build this form twice :)
|
Thanks for taking the time to look through this, I'll take another look with your suggestions and resubmit! |
lloydwheeler
added
some commits
May 2, 2017
|
Thanks for the pointers, I've spent some time refactoring the example.
Just a couple of other things:
Thanks! |
|
Force push to this PR to update it; please don't open a new one :-) |
jongold
approved these changes
May 9, 2017
View changes
Finally got to looking at this (sorry for the delay) - looks great!
Last thing - do you mind if I rename it to form validation?
|
No problem! Feel free to rename, by the time I got to the end of building the example I felt |
lloydwheeler commentedMay 1, 2017
PR for a new example combining DOM elements and React primitives. The idea was to see how easy it'd be for me to create of PoC of a (semi) working web-component and then render its various states to a sketch document.
The main hurdle was (as far as I'm aware) there not being a react-primitives textbox so that's the reasoning behind having a
TextBoxcomponent that hasprimitiveandwebcomponents inside it, with shared styles across the two.I imagine there's more than a few changes to be made here and there so let me know where I can look to improve things!