Upgrading the Import Account modal #17763

HowardBraham · 2023-02-15T06:33:30Z

When fixing these issues, I also tried to accomplish the following:

Use the Design System
Follow DRY principles (code length was reduced by 166 lines while adding more functionality)
Convert to React Hooks
Don't use Redux where it's not necessary

Fixes #16895 and #17719.
New error messages are in messages.json and are:

"You have entered a Secret Recovery Phrase (or mnemonic). To import an account here, you have to enter a private key, which is a hexadecimal string of length 64."

"This is not a valid private key. You have entered a hexadecimal string, but it must be 64 characters long."

"This is not a valid private key. You must enter a hexadecimal string of length 64."

Also helps with expectations around this multi-minute freeze when importing a JSON file #10113

github-actions · 2023-02-15T06:33:43Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

codecov-commenter · 2023-02-22T04:29:29Z

Codecov Report

Merging #17763 (9804d14) into develop (90ae498) will increase coverage by 0.07%.
The diff coverage is 29.63%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff             @@
##           develop   #17763      +/-   ##
===========================================
+ Coverage    62.96%   63.03%   +0.07%     
===========================================
  Files          909      910       +1     
  Lines        35419    35391      -28     
  Branches      8964     8964              
===========================================
+ Hits         22300    22308       +8     
+ Misses       13119    13083      -36

Impacted Files	Coverage Δ
.../components/app/asset-list-item/asset-list-item.js	`66.67% <ø> (ø)`
...mponent-library/form-text-field/form-text-field.js	`71.43% <ø> (ø)`
...omponents/component-library/help-text/help-text.js	`89.47% <ø> (ø)`
...i/pages/create-account/create-account.component.js	`0.00% <0.00%> (ø)`
...es/create-account/import-account/bottom-buttons.js	`0.00% <0.00%> (ø)`
...es/create-account/import-account/import-account.js	`0.00% <0.00%> (ø)`
ui/pages/create-account/import-account/json.js	`0.00% <0.00%> (ø)`
...pages/create-account/import-account/private-key.js	`0.00% <0.00%> (ø)`
ui/store/actions.ts	`39.34% <40.91%> (-0.07%)`	⬇️
app/scripts/account-import-strategies/index.js	`92.86% <84.21%> (-4.20%)`	⬇️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

NidhiKJha

Looking really good. Left a couple of UI suggestions

ui/pages/create-account/import-account/import-account.stories.js

ui/pages/create-account/new-account.stories.js

ui/pages/create-account/import-account/index.js

georgewrmarshall

This is looking great! I've made a few suggestions in #17905 - it requires a rebase of develop on this PR to see the changes effectively

ui/pages/create-account/import-account/bottom-buttons.js

app/scripts/account-import-strategies/index.js

ui/pages/create-account/import-account/bottom-buttons.js

ui/pages/create-account/import-account/index.js

ui/pages/create-account/import-account/import-account.stories.js

…ued to refine

metamaskbot · 2023-03-02T05:55:57Z

Builds ready [abbfb03]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1598 ± 52 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	95	138	115	12	6
		domContentLoaded	1305	1787	1563	99	48
		load	1305	1792	1598	108	52
		domInteractive	1305	1787	1563	99	48

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 433 bytes
ui: -122982 bytes
common: 121374 bytes

georgewrmarshall

This is looking fantastic! Left some small suggestions. Also not sure about the invisible character but would like to get further opinions

georgewrmarshall · 2023-03-02T20:48:18Z

ui/components/component-library/form-text-field/form-text-field.js

@@ -103,7 +103,6 @@ export const FormTextField = ({
    />
    {helpText && (
      <HelpText
-        error={error}


Oh thank you! Have a PR here for this #17939

ui/components/component-library/text/text.constants.js

ui/pages/create-account/import-account/import-account.js

ui/pages/create-account/import-account/bottom-buttons.js

metamaskbot · 2023-03-03T08:08:28Z

Builds ready [7d70de3]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1581 ± 45 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	98	152	118	14	7
		domContentLoaded	1294	1716	1537	86	41
		load	1295	1716	1581	94	45
		domInteractive	1294	1716	1537	86	41

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 433 bytes
ui: -122984 bytes
common: 121374 bytes

georgewrmarshall · 2023-03-04T02:27:07Z

ui/pages/create-account/import-account/bottom-buttons.js

+  const dispatch = useDispatch();
+
+  return (
+    <Box display={DISPLAY.FLEX}>


Suggested change

<Box display={DISPLAY.FLEX}>

<Box display={DISPLAY.FLEX} gap={4}>

I fixed this in a previous commit, I'm not sure how it came back 🤔

metamaskbot · 2023-03-04T12:24:12Z

Builds ready [104d553]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1598 ± 59 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	99	156	120	15	7
		domContentLoaded	1335	1819	1576	120	58
		load	1335	1819	1598	123	59
		domInteractive	1335	1819	1576	120	58

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 433 bytes
ui: -123030 bytes
common: 121374 bytes

georgewrmarshall

LGTM! 🚀

HowardBraham changed the title ~~Upgrade to Import Account modal~~ Upgrading the Import Account modal Feb 15, 2023

HowardBraham self-assigned this Feb 15, 2023

HowardBraham added the team-accounts label Feb 15, 2023

HowardBraham force-pushed the fix/import-account-error-messages branch 2 times, most recently from 36be5bc to 2846570 Compare February 17, 2023 00:06

HowardBraham marked this pull request as ready for review February 17, 2023 00:07

HowardBraham requested a review from a team as a code owner February 17, 2023 00:07

HowardBraham requested review from darkwing, georgewrmarshall and montelaidev February 17, 2023 00:07

HowardBraham force-pushed the fix/import-account-error-messages branch 3 times, most recently from 87a1638 to e791d40 Compare February 22, 2023 04:04

NidhiKJha reviewed Feb 23, 2023

View reviewed changes

georgewrmarshall mentioned this pull request Feb 25, 2023

Fix/import account error messages george suggestions #17905

Merged

georgewrmarshall reviewed Feb 25, 2023

View reviewed changes

HowardBraham force-pushed the fix/import-account-error-messages branch 5 times, most recently from 082b4f4 to 9804d14 Compare March 1, 2023 05:54

MetaMask deleted a comment from metamaskbot Mar 1, 2023

HowardBraham force-pushed the fix/import-account-error-messages branch from 9804d14 to c2925f3 Compare March 2, 2023 04:56

draft - fixing Import Account error messages (#16895)

ee09d5b

HowardBraham and others added 8 commits March 2, 2023 10:56

fixed lint and test errors

893815d

fixing e2e tests

2cb0fc7

responding to Monte's review

7a151b0

Fixes #17719

754d119

George updates

af227eb

Updates

1626dfa

Some suggestions

678b740

Merged suggestions from @georgewrmarshall and @NidhiKJha, then contin…

abbfb03

…ued to refine

HowardBraham force-pushed the fix/import-account-error-messages branch from c2925f3 to abbfb03 Compare March 2, 2023 05:26

MetaMask deleted a comment from metamaskbot Mar 2, 2023

georgewrmarshall reviewed Mar 2, 2023

View reviewed changes

Response to latest @georgewrmarshall review

4ff6bfc

HowardBraham force-pushed the fix/import-account-error-messages branch from ee95fc8 to 4ff6bfc Compare March 3, 2023 05:19

HowardBraham added 2 commits March 3, 2023 11:12

Merge branch 'develop' into fix/import-account-error-messages

2eff25c

fixed a test and bumped coverage targets

7d70de3

georgewrmarshall reviewed Mar 4, 2023

View reviewed changes

HowardBraham added 2 commits March 4, 2023 16:47

Merge branch 'develop' into fix/import-account-error-messages

3479ec7

eliminated the INVISIBLE_CHARACTER that was holding vertical space

104d553

georgewrmarshall approved these changes Mar 6, 2023

View reviewed changes

montelaidev approved these changes Mar 6, 2023

View reviewed changes

HowardBraham merged commit 694773f into develop Mar 6, 2023

HowardBraham deleted the fix/import-account-error-messages branch March 6, 2023 17:48

github-actions bot locked and limited conversation to collaborators Mar 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrading the Import Account modal #17763

Upgrading the Import Account modal #17763

HowardBraham commented Feb 15, 2023 •

edited

Loading

github-actions bot commented Feb 15, 2023

codecov-commenter commented Feb 22, 2023 •

edited

Loading

NidhiKJha left a comment

georgewrmarshall left a comment

metamaskbot commented Mar 2, 2023

georgewrmarshall left a comment

georgewrmarshall Mar 2, 2023

metamaskbot commented Mar 3, 2023

georgewrmarshall Mar 4, 2023

HowardBraham Mar 4, 2023

metamaskbot commented Mar 4, 2023

georgewrmarshall left a comment

	<Box display={DISPLAY.FLEX}>
	<Box display={DISPLAY.FLEX} gap={4}>

Upgrading the Import Account modal #17763

Upgrading the Import Account modal #17763

Conversation

HowardBraham commented Feb 15, 2023 • edited Loading

github-actions bot commented Feb 15, 2023

codecov-commenter commented Feb 22, 2023 • edited Loading

Codecov Report

NidhiKJha left a comment

Choose a reason for hiding this comment

georgewrmarshall left a comment

Choose a reason for hiding this comment

metamaskbot commented Mar 2, 2023

georgewrmarshall left a comment

Choose a reason for hiding this comment

georgewrmarshall Mar 2, 2023

Choose a reason for hiding this comment

metamaskbot commented Mar 3, 2023

georgewrmarshall Mar 4, 2023

Choose a reason for hiding this comment

HowardBraham Mar 4, 2023

Choose a reason for hiding this comment

metamaskbot commented Mar 4, 2023

georgewrmarshall left a comment

Choose a reason for hiding this comment

HowardBraham commented Feb 15, 2023 •

edited

Loading

codecov-commenter commented Feb 22, 2023 •

edited

Loading