update any instance of address validation to use InputAddress#125
update any instance of address validation to use InputAddress#125mannynotfound wants to merge 3 commits intomainfrom
Conversation
| isValid, | ||
| onChange, | ||
| className = "", | ||
| readOnly = false, |
There was a problem hiding this comment.
add className to add classes needed for situational positioning/styling and readOnly to render a disabled input
| <input | ||
| style={{ height: 48 }} | ||
| className="border-light rounded-sm column is-full p-2 mt-2" | ||
| className="border-light rounded-sm column is-full p-4" |
There was a problem hiding this comment.
moved p-2 => p-4 to match designs closer / be consistent throughout app
| <div style={{ position: "absolute", right: 17, top: 20 }}> | ||
| <div | ||
| className="is-flex is-align-items-center" | ||
| style={{ position: "absolute", right: 15, top: 0, height: "100%" }}> |
There was a problem hiding this comment.
use a container with 100% height thats align-items: center which should be more flexible than fixed absolute positioning
| )} | ||
| </div> | ||
| <InputAddress | ||
| className={addressValidClass} |
There was a problem hiding this comment.
It looks like this addressValidClass depends if the address is valid, so that logic can be moved to InputAddress?
This way we can ensure all the inputs have is-success/is-error class attached
|
Everything looks good, although I noticed that on your branch i'm unable to click
|
| const onOwnerAddressChange = (value, isValid, idx) => { | ||
| const newOwners = safeOwners.slice(0); | ||
| newOwners[idx].address = value; | ||
| newOwners[idx].isValid = isValid; | ||
| setSafeOwners([...newOwners]); | ||
| }; |
There was a problem hiding this comment.
Prefer following the style guide
| const onOwnerAddressChange = (value, isValid, idx) => { | |
| const newOwners = safeOwners.slice(0); | |
| newOwners[idx].address = value; | |
| newOwners[idx].isValid = isValid; | |
| setSafeOwners([...newOwners]); | |
| }; | |
| const onOwnerAddressChange = (value, isValid, idx) => { | |
| const newOwners = [...safeOwners] | |
| newOwners[idx].address = value; | |
| newOwners[idx].isValid = isValid; | |
| setSafeOwners(newOwners); | |
| }; |
| )} | ||
| </div> | ||
| </div> | ||
| <InputAddress |
There was a problem hiding this comment.
This input Address component is being replaced by addressDropdown. I think we can safely delete this file now https://github.com/DapperCollectives/VESSEL/pull/120/files#diff-e8e43eadee51d689965c6c2bd4a914f301aea66e12351d018baa6cd3567d1422
|
|
||
| const InputAddress = ({ web3, value, isValid, onChange }) => { | ||
| const InputAddress = ({ | ||
| web3, |
There was a problem hiding this comment.
Do we still need web3 as a prop when we expect isValid and onChange to handle the validation?
mannynotfound
force-pushed
the
manny/vsl-194
branch
from
131d99b to
1b7f4c2
Compare
mannynotfound
force-pushed
the
manny/vsl-194
branch
from
1b7f4c2 to
bb2ec7c
Compare
Small bug, should be updated now! |
4 participants
Description
We have several places in our app where we have an input for FLOW addresses which we want to validate against the FLOW blockchain and sometimes run additional validation logic on it. This PR moves all those custom
<input>to use our shared<InputAddress>so styling is consistent and to minimize places to update.Demo / Test Result