chore: MMI move custody component to TS #26096

zone-live · 2024-07-24T14:16:40Z

Description

Moves Custody component to TS.

Related issues

Fixes:

Manual testing steps

Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

github-actions · 2024-07-24T14:16:58Z

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.

greptile-apps

PR Summary

The Custody component has been migrated to TypeScript, enhancing type safety and maintainability.

ui/components/institutional/qr-code-modal/qr-code-modal.tsx: Updated QRCodeModalProps to allow custodianURL to be string | undefined.
ui/pages/institutional/custodian-list-view/custodian-list-view.tsx: Converted to TypeScript, defining custodianList prop as React.ReactNode[].
ui/pages/institutional/custody/custody.stories.tsx: Converted to TypeScript, updating imports and Redux store configuration.
ui/pages/institutional/custody/custody.test.tsx: Converted to TypeScript, adding type annotations and defining MockedFuseType.
ui/pages/institutional/manual-connect-custodian/manual-connect-custodian.tsx: Added TypeScript types for props and state, improving type safety.

_{7 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

The Custody component has been migrated to TypeScript, enhancing type safety and maintainability.

.circleci/config.yml: Removed test-e2e-swap-playwright job, potentially reducing test coverage for swap functionality.
app/scripts/background.js: Added logic for test environments, including fake keyring bridges and socket communication.
ui/components/institutional/confirm-remove-jwt-modal/*: Converted to TypeScript, ensuring type safety and better integration with Storybook.
ui/components/institutional/custody-confirm-link-modal/*: Converted to TypeScript, improving type safety and maintainability.
package.json: Updated dependencies and scripts to support TypeScript migration, ensuring compatibility and type safety.

_{118 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

The Custody component has been migrated to TypeScript, enhancing type safety and maintainability.

development/generate-rc-commits.js: Refactored to dynamically fetch authorTeams from teams.json in MetaMask-planning repository, improving maintainability.
ui/components/institutional/custody-confirm-link-modal/*: Converted to TypeScript, ensuring type safety and better integration with Storybook.
ui/pages/institutional/custody/custody.js: Migrated to TypeScript, enhancing type safety and maintainability.
ui/store/institutional/institution-actions.ts: Added TypeScript support, improving type safety and maintainability.
test/e2e/playwright/mmi/custodian-hooks/*: Updated to TypeScript, ensuring type safety and better integration with Playwright tests.

_{1 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

The Custody component has been migrated to TypeScript, enhancing type safety and maintainability.

app/_locales/en/messages.json: Added onboardingMetametricsPrivacyDescription key for MetaMetrics onboarding.
shared/lib/transaction-controller-utils.js: Introduced precision parameter in getSwapsTokensReceivedFromTxMeta for flexible rounding.
test/e2e/accounts/common.ts: Added delay to ensure popup window closure in installSnapSimpleKeyring.
ui/components/app/transaction-breakdown/transaction-breakdown.container.js: Added .toFixed() to calcTokenAmount calls for fixed-point formatting.
ui/pages/onboarding-flow/metametrics/metametrics.js: Refactored to use BoxComponent and added MetaMask privacy policy link.

_{18 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

metamaskbot · 2024-07-25T09:06:51Z

Builds ready [ca4348e]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-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, 5
- common: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (149 ± 179 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	66	154	93	23	11
		domContentLoaded	9	59	23	16	8
		load	40	1769	149	372	179
		domInteractive	9	59	23	16	8

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

codecov · 2024-07-25T09:07:15Z

Codecov Report

Attention: Patch coverage is 64.15094% with 19 lines in your changes missing coverage. Please review.

Project coverage is 69.69%. Comparing base (f9f3cff) to head (bf7ef25).
Report is 1 commits behind head on develop.

Files	Patch %	Lines
ui/pages/institutional/custody/custody.tsx	62.00%	19 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #26096      +/-   ##
===========================================
- Coverage    69.70%   69.69%   -0.01%     
===========================================
  Files         1409     1409              
  Lines        49788    49808      +20     
  Branches     13768    13785      +17     
===========================================
+ Hits         34702    34712      +10     
- Misses       15086    15096      +10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ui/pages/institutional/custody/custody.tsx

greptile-apps

PR Summary

(updates since last review)

The Custody component has been migrated to TypeScript, enhancing type safety and maintainability.

app/scripts/lib/transaction/metrics.ts: Added getIsConfirmationAdvancedDetailsOpen method for advanced transaction details.
ui/pages/confirmations/components/confirm/info/row/text-token-units.tsx: Introduced ConfirmInfoRowTextTokenUnits component with TypeScript.
ui/pages/confirmations/components/signature-request-original/signature-request-original.component.js: Added QueuedRequestsBannerAlert component for queued requests.
ui/pages/institutional/account-list/account-list.tsx: Converted CustodyAccountList to TypeScript, improving type safety.
ui/pages/confirmations/hooks/useQueuedConfirmationEvents.ts: Added useQueuedConfirmationsEvent hook for tracking queued confirmation events.

_{78 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

The Custody component has been migrated to TypeScript, enhancing type safety and maintainability.

ui/components/institutional/custody-labels/custody-labels.tsx: Converted to TypeScript; imports LabelItem type for consistency.
ui/pages/institutional/account-list/account-list.tsx: Converted to TypeScript; introduces types for props and uses optional chaining.
ui/pages/institutional/custody/custody.tsx: Converted to TypeScript; adds type definitions for various objects and state variables, improving robustness.

These changes improve type safety and maintainability but require thorough testing to ensure no runtime errors are introduced.

_{3 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

The Custody component has been migrated to TypeScript, enhancing type safety and maintainability.

ui/pages/institutional/custody/custody.tsx: Converted to TypeScript; adds type definitions for various objects and state variables, improving robustness.
Optional Properties: Certain properties in Account and AccountDetails types are now optional, which could lead to runtime errors if not handled properly.
Component Update: CustodyPage component updated to handle optional properties appropriately.
Testing: Thorough testing is required to ensure that the optional properties do not cause unexpected behavior.

_{1 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

metamaskbot · 2024-07-25T14:00:30Z

Builds ready [bf7ef25]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-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, 5
- common: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (140 ± 150 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	59	180	98	27	13
		domContentLoaded	9	64	24	16	8
		load	42	1495	140	311	150
		domInteractive	9	64	24	16	8

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

DDDDDanica · 2024-07-26T09:52:52Z

ui/components/institutional/custody-labels/custody-labels.tsx

-  key: string;
-  value: string;
-};
+import { LabelItem } from '../../../pages/institutional/custody/custody';


could we move the interface to a file that's more public accessible as institutional/custody/types;

Yes, I'll address in another PR. Ticket: https://consensyssoftware.atlassian.net/browse/MMI-5328

ui/pages/institutional/custody/custody.tsx

DDDDDanica · 2024-07-26T09:57:01Z

ui/pages/institutional/custody/custody.tsx

              onChange={(e) => setSelectAllAccounts(e)}
-              checked={isCheckBoxSelected}
+              checked={isCheckBoxSelected ?? undefined}


nit: you can leave it as isCheckBoxSelected because
true ?? undefined => true
undefined ?? undefined => undefined

DDDDDanica · 2024-07-26T09:58:00Z

ui/pages/institutional/custody/custody.tsx

+  const handleQrConnectionRequest = (message: string) => {
+    setQrConnectionRequest(message);
+  };


Nit:

Suggested change

const handleQrConnectionRequest = (message: string) => {

setQrConnectionRequest(message);

};

const _setQrConnectionRequest = (message: string) => {

setQrConnectionRequest(message);

};

…/MetaMask/metamask-extension into MMI-5306-custody-component-to-ts

sonarqubecloud · 2024-07-26T13:28:29Z

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

metamaskbot · 2024-07-26T13:56:43Z

Builds ready [fdb5821]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-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, 5
- common: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (620 ± 461 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	71	550	147	98	47
		domContentLoaded	9	114	39	26	13
		load	41	2832	620	961	461
		domInteractive	9	114	39	26	13

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

zone-live added 4 commits July 23, 2024 15:36

chore: move to ts

df52519

chore: lint and clean up

13766b0

chore: move test and storybook file to ts

17b80a0

chore: snapshot update

9a3278d

zone-live added the team-mmi PRs from the MMI team label Jul 24, 2024

zone-live requested a review from a team as a code owner July 24, 2024 14:16

Merge branch 'develop' into MMI-5306-custody-component-to-ts

d37c648

greptile-apps bot reviewed Jul 24, 2024

View reviewed changes

zone-live and others added 2 commits July 24, 2024 15:49

chore: updates

801fe2e

Merge branch 'develop' into MMI-5306-custody-component-to-ts

2d19c71

greptile-apps bot reviewed Jul 24, 2024

View reviewed changes

Merge branch 'develop' into MMI-5306-custody-component-to-ts

ca4348e

greptile-apps bot reviewed Jul 25, 2024

View reviewed changes

albertolive reviewed Jul 25, 2024

View reviewed changes

ui/pages/institutional/custody/custody.tsx Show resolved Hide resolved

albertolive previously approved these changes Jul 25, 2024

View reviewed changes

Merge branch 'develop' into MMI-5306-custody-component-to-ts

08d7349

zone-live dismissed albertolive’s stale review via 08d7349 July 25, 2024 11:52

greptile-apps bot reviewed Jul 25, 2024

View reviewed changes

chore: lint

dbf04dd

greptile-apps bot reviewed Jul 25, 2024

View reviewed changes

chore: lint

bf7ef25

greptile-apps bot reviewed Jul 25, 2024

View reviewed changes

zone-live requested a review from albertolive July 25, 2024 14:18

albertolive previously approved these changes Jul 26, 2024

View reviewed changes

DDDDDanica reviewed Jul 26, 2024

View reviewed changes

ui/pages/institutional/custody/custody.tsx Show resolved Hide resolved

DDDDDanica reviewed Jul 26, 2024

View reviewed changes

ui/pages/institutional/custody/custody.tsx Show resolved Hide resolved

DDDDDanica reviewed Jul 26, 2024

View reviewed changes

fix: PR comments

2dca5b9

albertolive dismissed their stale review via 2dca5b9 July 26, 2024 12:46

albertolive added 3 commits July 26, 2024 14:47

Merge branch 'develop' into MMI-5306-custody-component-to-ts

a833acf

fix: eslint issue

527a043

Merge branch 'MMI-5306-custody-component-to-ts' of https://github.com…

d19cd36

…/MetaMask/metamask-extension into MMI-5306-custody-component-to-ts

DDDDDanica approved these changes Jul 26, 2024

View reviewed changes

albertolive approved these changes Jul 26, 2024

View reviewed changes

Merge branch 'develop' into MMI-5306-custody-component-to-ts

fdb5821

vinistevam approved these changes Jul 26, 2024

View reviewed changes

albertolive merged commit 08c7333 into develop Jul 26, 2024
75 of 76 checks passed

albertolive deleted the MMI-5306-custody-component-to-ts branch July 26, 2024 15:33

github-actions bot locked and limited conversation to collaborators Jul 26, 2024

metamaskbot added the release-12.4.0 Issue or pull request that will be included in release 12.4.0 label Jul 26, 2024

gauthierpetetin added release-12.3.0 Issue or pull request that will be included in release 12.3.0 and removed release-12.4.0 Issue or pull request that will be included in release 12.4.0 labels Sep 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: MMI move custody component to TS #26096

chore: MMI move custody component to TS #26096

zone-live commented Jul 24, 2024

github-actions bot commented Jul 24, 2024

greptile-apps bot left a comment

greptile-apps bot left a comment

greptile-apps bot left a comment

greptile-apps bot left a comment

metamaskbot commented Jul 25, 2024

codecov bot commented Jul 25, 2024 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot left a comment

greptile-apps bot left a comment

metamaskbot commented Jul 25, 2024

DDDDDanica Jul 26, 2024

albertolive Jul 26, 2024

DDDDDanica Jul 26, 2024

DDDDDanica Jul 26, 2024

sonarqubecloud bot commented Jul 26, 2024

metamaskbot commented Jul 26, 2024

chore: MMI move custody component to TS #26096

chore: MMI move custody component to TS #26096

Conversation

zone-live commented Jul 24, 2024

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Jul 24, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

metamaskbot commented Jul 25, 2024

codecov bot commented Jul 25, 2024 • edited Loading

Codecov Report

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

metamaskbot commented Jul 25, 2024

DDDDDanica Jul 26, 2024

Choose a reason for hiding this comment

albertolive Jul 26, 2024

Choose a reason for hiding this comment

DDDDDanica Jul 26, 2024

Choose a reason for hiding this comment

DDDDDanica Jul 26, 2024

Choose a reason for hiding this comment

sonarqubecloud bot commented Jul 26, 2024

Quality Gate passed

metamaskbot commented Jul 26, 2024

codecov bot commented Jul 25, 2024 •

edited

Loading