Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(snaps): Add background color prop to Snaps UI Container component #29095

Merged
merged 7 commits into from
Jan 14, 2025

Conversation

david0xd
Copy link
Contributor

@david0xd david0xd commented Dec 11, 2024

Description

This PR adds backgroundColor property to Container component.
Background colors that can be used are predefined as: default and alternative.
This PR deliberately disables the backgroundColor feature for containers used within Transaction Insights pages (Snaps).

Open in GitHub Codespaces

Related issues

Fixes: MetaMask/snaps#2906
Related Snaps PR: MetaMask/snaps#2950

Manual testing steps

  1. Try installing and using a Snap with confirmation window or home page, with Container in its content with backgroundColor defined (e.g. <Container backgroundColor="default">).
  2. Try the same thing on transaction insights Snaps and confirm that the colors are not changing there.
  3. Confirm that the colors are as expected on all pages.

Some source code used for testing:

  • Source code used for confirmation content:
export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
  switch (request.method) {
    case 'hello':
      return snap.request({
        method: 'snap_dialog',
        params: {
          content: (
            <Container backgroundColor="default">
              <Box>
                <Section>
                  <Row label="From">
                    <Address address="0x1234567890123456789012345678901234567890" />
                  </Row>
                  <Row
                    label="To"
                    variant="warning"
                    tooltip="This address has been deemed dangerous."
                  >
                    <Address address="0x0000000000000000000000000000000000000000" />
                  </Row>
                </Section>
                <Form name="form">
                  <Box direction="horizontal">
                    <Field label="baz">
                      <Input name="baz" placeholder="Enter something..." />
                    </Field>
                    <Field label="foo">
                      <Selector title="bar" name="test">
                        <SelectorOption value="one">
                          <Card title="test" value="test" />
                        </SelectorOption>
                      </Selector>
                    </Field>
                  </Box>
                  <Field label="something_else">
                    <Selector title="bar" name="test2">
                      <SelectorOption value="one">
                        <Card title="test" value="test" />
                      </SelectorOption>
                    </Selector>
                  </Field>
                  <Box direction="horizontal">
                    <Field label="baz2">
                      <Input name="baz2" placeholder="Enter something..." />
                    </Field>
                    <Field label="foo3">
                      <Input name="foo3" placeholder="Enter something..." />
                    </Field>
                  </Box>
                  <Box direction="horizontal">
                    <Field label="Example Dropdown">
                      <Dropdown name="example-dropdown">
                        <Option value="option1">Option 1</Option>
                        <Option value="option2">Option 2</Option>
                        <Option value="option3">Option 3</Option>
                      </Dropdown>
                    </Field>
                    <Field label="Example RadioGroup">
                      <RadioGroup name="example-radiogroup">
                        <Radio value="option1">Option 1</Radio>
                        <Radio value="option2">Option 2</Radio>
                        <Radio value="option3">Option 3</Radio>
                      </RadioGroup>
                    </Field>
                  </Box>
                  <Field label="Example Dropdown full width">
                    <Dropdown name="example-dropdown-full">
                      <Option value="option1">Option 1</Option>
                      <Option value="option2">Option 2</Option>
                      <Option value="option3">Option 3</Option>
                    </Dropdown>
                  </Field>
                  <Field label="Example RadioGroup full width">
                    <RadioGroup name="example-radiogroup-full">
                      <Radio value="option11">Option 11</Radio>
                      <Radio value="option22">Option 22</Radio>
                      <Radio value="option33">Option 33</Radio>
                    </RadioGroup>
                  </Field>
                  <Field label="foo4">
                    <Input name="foo4" placeholder="Enter something..." />
                  </Field>
                  <Box direction="horizontal">
                    <Field label="baz3">
                      <Input name="baz3" placeholder="Enter something..." />
                    </Field>
                  </Box>
                  <Box direction="horizontal">
                    <Field label="Example Checkbox">
                      <Checkbox name="example-checkbox" label="Checkbox" />
                    </Field>
                    <Field label="Example Selector">
                      <Selector
                        name="example-selector"
                        title="Choose an option"
                        value="option1"
                      >
                        <SelectorOption value="option1">
                          <Card title="Option 1" value="option1" />
                        </SelectorOption>
                        <SelectorOption value="option2">
                          <Card title="Option 2" value="option2" />
                        </SelectorOption>
                        <SelectorOption value="option3">
                          <Card title="Option 3" value="option3" />
                        </SelectorOption>
                      </Selector>
                    </Field>
                  </Box>
                  <Field label="Example FileInput 2">
                    <FileInput name="file-input-2" />
                  </Field>
                  <Field label="Example Checkbox Full field">
                    <Checkbox name="example-checkbox-full" label="Checkbox" />
                  </Field>
                  <Box direction="horizontal">
                    <Field label="baz4">
                      <Input name="baz4" placeholder="Enter something..." />
                    </Field>
                    <Field label="Example FileInput">
                      <FileInput name="file-input-1" />
                    </Field>
                  </Box>
                  <Box direction="horizontal">
                    <Field label="baz5">
                      <Input name="baz5" placeholder="Enter something..." />
                    </Field>
                    <Field label="FileInput Compact">
                      <FileInput name="file-input-3" compact={true} />
                    </Field>
                  </Box>
                  <Field label="FileInput Compact 2 full">
                    <FileInput name="file-input-4" compact={true} />
                  </Field>
                </Form>
              </Box>
            </Container>
          ),
        },
      });
    default:
      throw new Error('Method not found.');
  }
};

Screenshots/Recordings

Before

Note: Changing container colors was not possible before.
Screenshot 2024-12-11 at 14 06 34
Screenshot 2024-12-11 at 14 06 59

After

Screenshot 2025-01-13 at 13 34 54
Screenshot 2025-01-13 at 13 38 00
Screenshot 2025-01-13 at 13 38 32
Screenshot 2025-01-13 at 13 41 11
Screenshot 2025-01-13 at 13 43 54
Screenshot 2025-01-13 at 13 44 25

Pre-merge author checklist

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.

@david0xd david0xd added the team-snaps-platform Snaps Platform team label Dec 11, 2024
@david0xd david0xd self-assigned this Dec 11, 2024
Copy link
Contributor

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.

@metamaskbot
Copy link
Collaborator

Builds ready [0f5f5d3]
Page Load Metrics (1675 ± 43 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1500184116859948
domContentLoaded1487179316508943
load1549183716758943
domInteractive24172564321
backgroundConnect997282412
firstReactRender16101483416
getState566192110
initialActions01000
loadScripts1115137012367335
setupStore67315189
uiStartup174724722035231111
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 376 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [fdf9081]
Page Load Metrics (1629 ± 43 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1525184916369546
domContentLoaded1481178916048842
load1524180716298943
domInteractive24218524622
backgroundConnect985262110
firstReactRender1691562813
getState595152110
initialActions01000
loadScripts1068137811808240
setupStore791212412
uiStartup17292365196919795
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 504 Bytes (0.01%)
  • common: -4.81 KiB (-0.06%)

@metamaskbot
Copy link
Collaborator

Builds ready [406ca2d]
Page Load Metrics (1811 ± 85 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15902271181518187
domContentLoaded15592249177817383
load15982274181117785
domInteractive27110442211
backgroundConnect880362211
firstReactRender16103623517
getState691333014
initialActions01000
loadScripts11371732132615273
setupStore78015209
uiStartup183931732255320154
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 1.03 KiB (0.01%)
  • common: -4.81 KiB (-0.06%)

@david0xd david0xd force-pushed the dd/add-snap-ui-container-bg branch from 88e1224 to 7004169 Compare January 13, 2025 11:01
@metamaskbot
Copy link
Collaborator

Builds ready [21ff2f2]
Page Load Metrics (1465 ± 25 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1354157414745627
domContentLoaded1350154714505024
load1354155714655325
domInteractive17198464019
backgroundConnect74118136
firstReactRender1592342613
getState45110136
initialActions01000
loadScripts994116510704823
setupStore67600
uiStartup1547199816799646
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 539 Bytes (0.01%)
  • common: 0 Bytes (0.00%)

@eriknson
Copy link
Member

Lgtm!

david0xd added a commit to MetaMask/snaps that referenced this pull request Jan 13, 2025
This PR adds `backgroundColor` property to `Container` component.

Related task: #2906
Related extension PR:
MetaMask/metamask-extension#29095

### Notes
- Background colors that can be used are predefined as: `default` and
`alternative`.
- Background colors are meant to mirror `backgroundDefault` and
`backgroundAlternative` colors from MetaMask extension design system
(https://github.com/MetaMask/metamask-extension/blob/main/ui/helpers/constants/design-system.ts#L54).
- Color names are simplified on the Snaps side to make it easier for
developers to use.

### Examples (experiments with extension integration)

Confirmation example:
![Screenshot 2024-12-11 at 13 14
02](https://github.com/user-attachments/assets/f2b77202-d9c4-403e-87a1-ad36d44299c9)

Source code used for content:
```typescript
      return snap.request({
        method: 'snap_dialog',
        params: {
          content: (
            <Container backgroundColor="default">
              <Box>
                <Text>Testing container background.</Text>
                <Button variant="primary">Primary button</Button>
                <Button variant="destructive">Destructive button</Button>
                <Button disabled={true}>Disabled button</Button>
              </Box>
              <Footer>
                <Button>Accept</Button>
                <Button name="footer_button">Cancel</Button>
              </Footer>
            </Container>
          ),
        },
      });
```

Transaction insight example where background color is deliberately
ignored:
![Screenshot 2024-12-11 at 13 07
40](https://github.com/user-attachments/assets/b7b3a593-8407-4f92-a629-edd85c8f88dc)
Source code used for content:
```typescript
    return {
      content: (
        <Container backgroundColor="alternative">
          <Box>
            <Text>Testing container background on transaction insight.</Text>
            <Text>Normal transaction.</Text>
          </Box>
        </Container>
      ),
      severity: SeverityLevel.Critical,
    };
```
@david0xd david0xd force-pushed the dd/add-snap-ui-container-bg branch from 21ff2f2 to 40e83f8 Compare January 14, 2025 11:53
@david0xd david0xd changed the base branch from main to fb/snaps-bump-v82 January 14, 2025 11:53
@david0xd david0xd force-pushed the dd/add-snap-ui-container-bg branch from 40e83f8 to f5bc652 Compare January 14, 2025 11:53
@david0xd david0xd marked this pull request as ready for review January 14, 2025 11:53
@david0xd david0xd requested review from a team as code owners January 14, 2025 11:53
@FrederikBolding FrederikBolding merged commit 603bf8d into fb/snaps-bump-v82 Jan 14, 2025
52 of 72 checks passed
@FrederikBolding FrederikBolding deleted the dd/add-snap-ui-container-bg branch January 14, 2025 15:57
@github-actions github-actions bot locked and limited conversation to collaborators Jan 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
team-snaps-platform Snaps Platform team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow background prop on Container
4 participants