-
Notifications
You must be signed in to change notification settings - Fork 193
Conversation
Codecov Report
@@ Coverage Diff @@
## 3.0-react #2492 +/- ##
=============================================
Coverage 100.00% 100.00%
=============================================
Files 24 266 +242
Lines 142 2698 +2556
Branches 18 494 +476
=============================================
+ Hits 142 2698 +2556
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nothing major that I can see. Possibly just the validation of address imports
src/app/hooks/env.ts
Outdated
|
||
return useMemo(() => { | ||
if (env) { | ||
return env.availableNetworks().map((network) => network); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the map required since it returns itself? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexbarnsley what do you think about returning?
name: `${network.ticker()} - ${network.name()}`,
icon: `${network.coin().charAt(0).toUpperCase()}${network.coin().slice(1).toLowerCase()}`,
coin: network.coin(),
network: network.name().toLowerCase(),
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexbarnsley I updated the hook: f8f4ce6
@@ -43,106 +77,119 @@ const ImportWallet = ({ networks, onSubmit }: Props) => { | |||
// TODO: Change to InputAddress | |||
<FormField name="address"> | |||
<FormLabel label="Address" /> | |||
<Input data-testid="import-wallet__address-input" ref={register} /> | |||
<Input ref={register({ required: "Address is required" })} data-testid="ImportWallet__address-input" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worth incorporating the InputAddres now, as I think additional validation is required here, I get an error in the console if I put an invalid address. Or if it's ok with @faustbrian, do another PR next with that component and then update it here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah! I can send another PR and update this 👍
Summary
Task: Implement Wallet Import
Checklist