Conversation
# Conflicts: # src/App.tsx # src/components/Layout/Fullscreen.tsx # src/components/mnemonicPhrase/DisplayMnemonicPhrase.tsx # src/views/MnemonicPhrase/index.tsx
| const dispatch = useDispatch(); | ||
| const words = useRef(shuffle(mnemonicPhrase.split(" "))); | ||
| const words = shuffle(mnemonicPhrase.split(" ")); | ||
| const wordState = useRef( |
There was a problem hiding this comment.
this form starts to get more complicated bc we're introducing a "clear field" button. we need to create an object to store all the values of each form input so we can clear em all at once when needed
There was a problem hiding this comment.
this is the initial state. setting all the checkboxes to false (AKA, not checked)
There was a problem hiding this comment.
It might be a good idea to switch to Formik or another form tool sooner rather than later. I know that Formik gives you "reset form" out of the box
There was a problem hiding this comment.
yup, been looking at some form libraries. The next iteration of this will be using one
| words.reduce( | ||
| (obj, current, i) => ({ | ||
| ...obj, | ||
| [`${current}-${i}`]: false, |
There was a problem hiding this comment.
we need to create a unique key bc evidently a mnemonic phrase can repeat the same word at different pts
|
|
||
| const wordBubbles = () => | ||
| words.current.map((word) => ( | ||
| wordStateArr.map(([wordKey, value]) => ( |
There was a problem hiding this comment.
grab each "word/checked" state and create a checkbox from it
| wordStateArr.map(([wordKey, value]) => ( | ||
| <CheckButton | ||
| onChange={(e) => { | ||
| setCheckboxState((prev) => ({ ...prev, [wordKey]: !value })); |
There was a problem hiding this comment.
update the form state to track changing values
src/components/Layout/Fullscreen.tsx
Outdated
| icon: [src, alt], | ||
| children, | ||
| }: { | ||
| back?: () => void; |
There was a problem hiding this comment.
I like to name function props with the naming pattern on{Action} (in this case, onBack) to make it obvious that it's a function that's called when something happens. Otherwise, back could easily seem like a React node, or a bool, or anything else. In fact, we should add this to the product conventions style guide...
| <Header /> | ||
|
|
||
| <Wrapper> | ||
| <H1>Woo, you're in!</H1> |
There was a problem hiding this comment.
It's worth getting used to using correct typographic characters everywhere there's user-facing text: this should be an apostrophe, ’, not a single quote, '. It's annoying but looks way better, and the only way to get used to it is to always do it for ellipses, single and double quote marks, etc. See also https://github.com/stellar/product-conventions/blob/master/STYLEGUIDE.md#use-the-right-symbols
There was a problem hiding this comment.
Also, “‘”’ can by typed on a mac with alt-[, -] and alt-shift-[, -]. … with alt-;
src/components/Layout/Fullscreen.tsx
Outdated
| const Fullscreen = ({ | ||
| back, | ||
| header, | ||
| icon: [src, alt], |
There was a problem hiding this comment.
This icon format is really specific. I can see an inattentive developer making any of these mistakes the way this is designed:
- Passing a React node
- Passing a string just for the src
- Reversing the order of src and alt
- Not providing an alt (which obviously is fine, but here it's typed to be required)
Some ways to add some affordance to this design:
- Take an object with the shape
{ src: string; alt?: string; } - Take a ReactNode
- ???
src/components/Layout/Fullscreen.tsx
Outdated
| import { Button } from "styles/Basics"; | ||
| import Header from "./Header"; | ||
|
|
||
| const H1 = styled.h1` |
There was a problem hiding this comment.
I like to name styled components a little more deliberately:
- I suffix all local styled components with
Elso they're easily distinguished from imported components (see https://github.com/stellar/product-conventions/blob/master/STYLEGUIDE.md#naming-1) - I prefer names that represent what they are (i.e. HeaderEl, TitleEl, etc.) rather than something implementation-specific, like their tag or their order on the page. That stuff changes frequently.
3 participants
No description provided.