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.

Sign up for GitHub

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

Jump to bottom

Allow background prop on Container #2906

Open
GuillaumeRx opened this issue Nov 25, 2024 · 4 comments · Fixed by MetaMask/metamask-extension#29095
Open

Allow background prop on Container #2906

GuillaumeRx opened this issue Nov 25, 2024 · 4 comments · Fixed by MetaMask/metamask-extension#29095
Assignees
@david0xd

Comments

@GuillaumeRx
Copy link
Contributor

GuillaumeRx commented Nov 25, 2024 •
edited by FrederikBolding
Loading

We should make the background configurable for Container to allow Snaps to choose between white and gray background. There may be places where we want to not honor this background choice, e.g. insights.
@ajeetd
Copy link

ajeetd commented Nov 25, 2024 •
edited
Loading

Support for this should be for: Pre-installed Snaps && hideSnapsBranding

Do a review of how components look on white background.

@ajeetd
Copy link

ajeetd commented Dec 2, 2024

Confirmations background for Bitcoin/Solana?

@FrederikBolding FrederikBolding changed the title Let hideSnapBranding make the dialog background white Allow background prop on Container Dec 2, 2024
@ajeetd
Copy link

ajeetd commented Dec 5, 2024

Implement and fix PoC as needed.

@david0xd david0xd self-assigned this Dec 9, 2024
This was referenced Dec 11, 2024
Merged
Merged
@ajeetd
Copy link

ajeetd commented Dec 11, 2024

Erik following up with DS on additional color.

david0xd added a commit that referenced this issue Jan 13, 2025
@david0xd
Add backgroundColor property to Container component (#2950)
6069bb0 
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,
    };
```
FrederikBolding pushed a commit to MetaMask/metamask-extension that referenced this issue Jan 14, 2025
@david0xd
feat(snaps): Add background color prop to Snaps UI Container component (
603bf8d 
#29095)

## **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](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29095?quickstart=1)

## **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:
```typescript
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](https://github.com/user-attachments/assets/aa7fbeb9-37ab-48f9-9613-2a76fb64960b)
![Screenshot 2024-12-11 at 14 06
59](https://github.com/user-attachments/assets/a06b620a-5755-4297-96a5-676bafb60638)

### **After**
![Screenshot 2025-01-13 at 13 34
54](https://github.com/user-attachments/assets/28486134-b5cf-42c9-ac83-eada133a00ad)
![Screenshot 2025-01-13 at 13 38
00](https://github.com/user-attachments/assets/8ecd5169-a49e-4ba0-9cc2-baa3a92a2d75)
![Screenshot 2025-01-13 at 13 38
32](https://github.com/user-attachments/assets/c674aa5c-0b52-42a7-a58a-b894dcddeb58)
![Screenshot 2025-01-13 at 13 41
11](https://github.com/user-attachments/assets/a1b5ca36-e833-40a2-84c7-06e31af57d8b)
![Screenshot 2025-01-13 at 13 43
54](https://github.com/user-attachments/assets/cf5436e1-6626-4641-933d-5a02d031bb65)
![Screenshot 2025-01-13 at 13 44
25](https://github.com/user-attachments/assets/f598d930-ebee-473a-aea8-5132ec60114c)

## **Pre-merge author checklist**
- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] 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](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@david0xd david0xd

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(snaps): Add background color prop to Snaps UI Container component MetaMask/metamask-extension
4 participants
@david0xd @GuillaumeRx @ajeetd @eriknson