feat: add `Delineator` component #25610

OGPoyraz · 2024-07-01T12:29:39Z

Description

This PR aims to add Delineator component which heavily influenced by SnapsDelineator with small visual differences and decoupled from snaps context.

Related issues

Fixes: #24995

Manual testing steps

This component has no usage for now, hence it could be seen in the storybook.

Run yarn storybook
Search for Delineator component to see it visually

Screenshots/Recordings

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.

jpuri · 2024-07-01T12:54:47Z

ui/components/ui/delineator/utils.ts

+    default:
+      return TextColor.textAlternative;
+  }
+};


Nit: Above function can be a simple object key -> value could be DelineatorType -> TextColor

Good call but then default value needs to be determined by an || operator.

const textColorMap = { [DelineatorType.Error]: TextColor.errorDefault, }; /// usage textColorMap[type] || TextColor.textAlternative;

jpuri · 2024-07-01T12:56:57Z

ui/components/ui/delineator/delineator.tsx

+          {overrideTextComponentColorByType({
+            component: headerComponent,
+            type,
+          })}


Here is it not possible to pass color prop without cloning the component ?

I am actually not fan of this but there is no way to override that property rather than this.

Normally the parent needs to set the <Text> component color becuase they also set the type. However this is an extra security to override whatever send in headerComponent

Do you think we should remove this and let parent set the color?

// Normal usage <Delineator headerComponent={ <Text color={TextColor.textAlternative} marginLeft={1} variant={TextVariant.bodySm} > Insights from <b>Forta</b> </Text> } iconName={IconName.Wallet} > Content </Delineator> // Error usage - Actually expected the text color to be send in error color <Delineator type="error" headerComponent={ <Text color={TextColor.errorDefault} marginLeft={1} variant={TextVariant.bodySm} > Insights from <b>Forta</b> </Text> } iconName={IconName.Wallet} > Content </Delineator>

I agree that this component setting the colour is best since it enforces the consistent styling, plus believe cloning is best practice to override child properties.

jpuri · 2024-07-01T12:58:11Z

ui/components/ui/delineator/utils.ts

+      };
+  }
+  return { ...defaultIconProps, ...iconProps };
+};


Is it possible to have icon without iconsProps as inverseIconColorProp ? backgroundColor seems to be only property changing above.

Good call, adrdressed this in 7b0a644

jpuri · 2024-07-01T13:03:01Z

ui/components/ui/delineator/delineator.tsx

+  onExpandChange,
+  type,
+  wrapperBoxProps,
+}) => {


Will it be required to have props isExpanded, onExpandChange one level above. Or they can be part of this component itself ?

I think it will create unnecessary state management overload where this component is used if we leave the expand state to parent. I've added onExpandChange because I saw that SnapsDelineator needs such callback (probably for metrics etc)

ui/components/ui/delineator/delineator.tsx

matthewwalsh0 · 2024-07-04T09:16:49Z

ui/components/ui/delineator/delineator.tsx

+          {overrideTextComponentColorByType({
+            component: headerComponent,
+            type,
+          })}


I agree that this component setting the colour is best since it enforces the consistent styling, plus believe cloning is best practice to override child properties.

ui/components/ui/delineator/delineator.tsx

metamaskbot · 2024-07-10T15:02:19Z

Builds ready [12500e4]

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, 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 (146 ± 151 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	61	147	102	23	11
		domContentLoaded	9	60	27	14	7
		load	41	1514	146	315	151
		domInteractive	9	60	27	14	7

Bundle size diffs

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

codecov · 2024-07-10T15:02:56Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.81%. Comparing base (5ee57a6) to head (d5f38f1).
Report is 26 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #25610      +/-   ##
===========================================
+ Coverage    69.77%   69.81%   +0.03%     
===========================================
  Files         1376     1380       +4     
  Lines        48403    48628     +225     
  Branches     13348    13407      +59     
===========================================
+ Hits         33773    33945     +172     
- Misses       14630    14683      +53

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

jpuri

looks good

ui/components/ui/delineator/delineator.tsx

sonarcloud · 2024-07-11T07:42:37Z

Quality Gate passed

Issues
5 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-11T08:09:22Z

Builds ready [d5f38f1]

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, 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 (155 ± 179 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	69	138	99	19	9
		domContentLoaded	9	55	24	12	6
		load	42	1781	155	374	179
		domInteractive	9	55	24	12	6

Bundle size diffs

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

add delineator component

4d9d44e

OGPoyraz added the team-confirmations Push issues to confirmations team label Jul 1, 2024

OGPoyraz requested a review from a team as a code owner July 1, 2024 12:29

OGPoyraz linked an issue Jul 1, 2024 that may be closed by this pull request

Build expandable UI component to allow users to add content to the page, keeping the design aligned #24995

Closed

jpuri reviewed Jul 1, 2024

View reviewed changes

OGPoyraz added 2 commits July 2, 2024 11:14

Fix getIconPropsByType switch

7b0a644

Fix lint

7340067

OGPoyraz force-pushed the 24995-build-delineator branch from 45de3ec to 7340067 Compare July 2, 2024 10:43

OGPoyraz added 3 commits July 2, 2024 13:03

Merge branch 'develop' into 24995-build-delineator

795e57d

Merge branch 'develop' into 24995-build-delineator

383e638

Merge branch 'develop' into 24995-build-delineator

ef66086

matthewwalsh0 reviewed Jul 4, 2024

View reviewed changes

OGPoyraz added 3 commits July 10, 2024 14:28

Fix suggestions

d6c7cb8

Merge branch 'develop' into 24995-build-delineator

f3b9e10

Fix lint

12500e4

pedronfigueiredo previously approved these changes Jul 10, 2024

View reviewed changes

jpuri previously approved these changes Jul 11, 2024

View reviewed changes

matthewwalsh0 previously approved these changes Jul 11, 2024

View reviewed changes

ui/components/ui/delineator/delineator.tsx Outdated Show resolved Hide resolved

Rename callback function

d5f38f1

OGPoyraz dismissed stale reviews from matthewwalsh0, jpuri, and pedronfigueiredo via d5f38f1 July 11, 2024 07:40

matthewwalsh0 approved these changes Jul 11, 2024

View reviewed changes

jpuri approved these changes Jul 11, 2024

View reviewed changes

OGPoyraz merged commit 5f75083 into develop Jul 11, 2024
73 checks passed

OGPoyraz deleted the 24995-build-delineator branch July 11, 2024 08:48

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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `Delineator` component #25610

feat: add `Delineator` component #25610

OGPoyraz commented Jul 1, 2024 •

edited by github-actions bot

Loading

jpuri Jul 1, 2024

OGPoyraz Jul 2, 2024

jpuri Jul 1, 2024

OGPoyraz Jul 2, 2024 •

edited

Loading

matthewwalsh0 Jul 4, 2024

jpuri Jul 1, 2024 •

edited

Loading

OGPoyraz Jul 2, 2024

jpuri Jul 1, 2024

OGPoyraz Jul 2, 2024 •

edited

Loading

matthewwalsh0 Jul 4, 2024

metamaskbot commented Jul 10, 2024

codecov bot commented Jul 10, 2024 •

edited

Loading

jpuri left a comment

sonarcloud bot commented Jul 11, 2024

metamaskbot commented Jul 11, 2024

feat: add Delineator component #25610

feat: add Delineator component #25610

Conversation

OGPoyraz commented Jul 1, 2024 • edited by github-actions bot Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Pre-merge author checklist

Pre-merge reviewer checklist

jpuri Jul 1, 2024

Choose a reason for hiding this comment

OGPoyraz Jul 2, 2024

Choose a reason for hiding this comment

jpuri Jul 1, 2024

Choose a reason for hiding this comment

OGPoyraz Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

matthewwalsh0 Jul 4, 2024

Choose a reason for hiding this comment

jpuri Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

OGPoyraz Jul 2, 2024

Choose a reason for hiding this comment

jpuri Jul 1, 2024

Choose a reason for hiding this comment

OGPoyraz Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

matthewwalsh0 Jul 4, 2024

Choose a reason for hiding this comment

metamaskbot commented Jul 10, 2024

codecov bot commented Jul 10, 2024 • edited Loading

Codecov Report

jpuri left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Jul 11, 2024

Quality Gate passed

metamaskbot commented Jul 11, 2024

feat: add `Delineator` component #25610

feat: add `Delineator` component #25610

OGPoyraz commented Jul 1, 2024 •

edited by github-actions bot

Loading

OGPoyraz Jul 2, 2024 •

edited

Loading

jpuri Jul 1, 2024 •

edited

Loading

OGPoyraz Jul 2, 2024 •

edited

Loading

codecov bot commented Jul 10, 2024 •

edited

Loading