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

Address component #2758

Closed
eriknson opened this issue Sep 25, 2024 · 4 comments · Fixed by MetaMask/metamask-extension#27798
Closed

Address component #2758

eriknson opened this issue Sep 25, 2024 · 4 comments · Fixed by MetaMask/metamask-extension#27798
Assignees
@david0xd
Labels
team-snaps

Comments

@eriknson
Copy link
Member

eriknson commented Sep 25, 2024 •
edited
Loading

Let's introduce a component to parse any arbitrary account address so it can be displayed in a variety of ways depending on the props. E.g. "Gaming Account 1" or the truncated address (e.g. "0xEqT...83x")

Specification

  • Unformatted so it can be populated within other components such as Cell, Row, plain text, etc

Props

  • address: CAIP10Address
  • truncate: boolean
  • displayName: boolean
  • avatar: boolean

Figma
@david0xd david0xd self-assigned this Sep 30, 2024
@eriknson eriknson changed the title Address component to display user-owned accounts via their client names Address component Oct 4, 2024
@danroc
Copy link
Contributor

danroc commented Oct 7, 2024

I think we should add a requirement for the address to be a CAIP-10 address. We may also need the chain ID information if we decide to customize the address display per chain.

@danroc
Copy link
Contributor

danroc commented Oct 7, 2024

The accountId is a good addition if account Snaps will use this component to display managed accounts, as it's the preferred way to identify managed accounts within MetaMask and account Snaps.

@FrederikBolding
Copy link
Member

I think we should add a requirement for the address to be a CAIP-10 address. We may also need the chain ID information if we decide to customize the address display per chain.

The address prop already supports CAIP-10 addresses, so we should be good on that front.

The accountId is a good addition if account Snaps will use this component to display managed accounts, as it's the preferred way to identify managed accounts within MetaMask and account Snaps.

Curious if it wouldn't be better for the Snap to explicitly choose the CAIP-10 address it wants to display, instead of us maintaining the logic to choose that based on an account ID. At least that seems to match better what we were planning for AccountSelector etc. WDYT?

@david0xd david0xd mentioned this issue Oct 11, 2024
Merged
7 tasks
@danroc
Copy link
Contributor

danroc commented Oct 14, 2024

Curious if it wouldn't be better for the Snap to explicitly choose the CAIP-10 address it wants to display, instead of us maintaining the logic to choose that based on an account ID. At least that seems to match better what we were planning for AccountSelector etc. WDYT?

Agreed, it makes sense for the Address component to expect an address. I think I confused it with the Account component, which could have an accountId prop.

@david0xd david0xd mentioned this issue Oct 14, 2024
Merged
david0xd added a commit that referenced this issue Oct 16, 2024
@david0xd
Add updated props for address component (#2833)
a916c19 
Update props of address component to get it work in more flexible way.

Integration PR for `metamask-extension`:
MetaMask/metamask-extension#27798

Related ticket: #2758
FrederikBolding pushed a commit to MetaMask/metamask-extension that referenced this issue Nov 4, 2024
@david0xd @GuillaumeRx
feat: Add changes to Snaps Address UI component (#27798)
df64b2b 
## **Description**
Add changes to Snaps Address UI component to support more flexible use
cases.

This PR needs release of the `snaps-sdk` after merging:
MetaMask/snaps#2833

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27798?quickstart=1)

## **Related issues**
Fixes: MetaMask/snaps#2758

## **Manual testing steps**
1. Use custom Snap build from template-snap-monorepo and add few
varieties of address component to test new capabilities.
2. Make sure UI is looking good.
3. Make sure that there are no regressions.

## **Screenshots/Recordings**
### **Before**
![Screenshot 2024-10-11 at 18 12
06](https://github.com/user-attachments/assets/4d498bd6-0ef0-445c-90ab-40918e63b421)

### **After**
![Screenshot 2024-10-15 at 14 34
16](https://github.com/user-attachments/assets/792244b6-300d-45e2-b7e1-91bbcb93a9a6)
![Screenshot 2024-10-15 at 14 34
28](https://github.com/user-attachments/assets/9c6776e1-f6b6-4888-864b-b95621e83069)
![Screenshot 2024-10-15 at 14 34
47](https://github.com/user-attachments/assets/054a9127-ce9a-43d9-ab06-61c4099990a6)
![Screenshot 2024-10-15 at 14 35
10](https://github.com/user-attachments/assets/4c829486-2195-4f14-947e-cbc3572eb822)

### Snap code used for testing
#### (_Might be useful for QA_)
```typescript
export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
  switch (request.method) {
    case 'hello':
      return snap.request({
        method: 'snap_dialog',
        params: {
          type: 'confirmation',
          content: (
            <Box>
              <Heading size="lg">Some unknown address</Heading>

              <Text>Default address</Text>
              <Address address="0x1acC6721CF7642Eb37651F8019AC9a785C064002" />

              <Text>Custom address (displayName=true)</Text>
              <Address
                address="0x1acC6721CF7642Eb37651F8019AC9a785C064002"
                displayName={true}
              />

              <Text>Custom address (avatar=false)</Text>
              <Address
                address="0x1234567890123456789012345678901234567890"
                avatar={false}
              />

              <Text>Custom address (displayName=true, avatar=false)</Text>
              <Address
                address="0x1234567890123456789012345678901234567890"
                displayName={true}
                avatar={false}
              />

              <Text>Custom address (truncate=false)</Text>
              <Address
                address="0x1234567890123456789012345678901234567890"
                truncate={false}
              />

              <Text>Custom address (truncate=false, avatar=false)</Text>
              <Address
                address="0x1234567890123456789012345678901234567890"
                truncate={false}
                avatar={false}
              />

              <Heading size="lg">My address</Heading>

              <Text>Default address</Text>
              <Address address="0xBc641D258aA0e90F70AaDbcC4dE2D58d88029713" />

              <Text>Custom address (displayName=true)</Text>
              <Address
                address="0xBc641D258aA0e90F70AaDbcC4dE2D58d88029713"
                displayName={true}
              />

              <Text>Custom address (avatar=false)</Text>
              <Address
                address="0xBc641D258aA0e90F70AaDbcC4dE2D58d88029713"
                avatar={false}
              />

              <Text>Custom address (displayName=true, avatar=false)</Text>
              <Address
                address="0xBc641D258aA0e90F70AaDbcC4dE2D58d88029713"
                displayName={true}
                avatar={false}
              />

              <Text>Custom address (truncate=false)</Text>
              <Address
                address="0xBc641D258aA0e90F70AaDbcC4dE2D58d88029713"
                truncate={false}
              />

              <Text>Custom address (truncate=false, avatar=false)</Text>
              <Address
                address="0xBc641D258aA0e90F70AaDbcC4dE2D58d88029713"
                truncate={false}
                avatar={false}
              />

              <Heading size="lg">Contact address</Heading>

              <Text>Default address</Text>
              <Address address="0x1acC6721CF7642Eb37651F8019AC9a785C064002" />

              <Text>Custom address (displayName=true)</Text>
              <Address
                address="0x1acC6721CF7642Eb37651F8019AC9a785C064002"
                displayName={true}
              />

              <Text>Custom address (avatar=false)</Text>
              <Address
                address="0x1acC6721CF7642Eb37651F8019AC9a785C064002"
                avatar={false}
              />

              <Text>Custom address (displayName=true, avatar=false)</Text>
              <Address
                address="0x1acC6721CF7642Eb37651F8019AC9a785C064002"
                displayName={true}
                avatar={false}
              />

              <Text>Custom address (truncate=false)</Text>
              <Address
                address="0x1acC6721CF7642Eb37651F8019AC9a785C064002"
                truncate={false}
              />

              <Text>Custom address (truncate=false, avatar=false)</Text>
              <Address
                address="0x1acC6721CF7642Eb37651F8019AC9a785C064002"
                truncate={false}
                avatar={false}
              />
            </Box>
          ),
        },
      });
    default:
      throw new Error('Method not found.');
  }
};
```

## **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/develop/.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/develop/.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.

---------

Co-authored-by: Guillaume Roux <[email protected]>
FrederikBolding pushed a commit to MetaMask/metamask-extension that referenced this issue Nov 4, 2024
@david0xd @GuillaumeRx @FrederikBolding
feat: Add changes to Snaps Address UI component (#27798)
093caf7 
## **Description**
Add changes to Snaps Address UI component to support more flexible use
cases.

This PR needs release of the `snaps-sdk` after merging:
MetaMask/snaps#2833

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27798?quickstart=1)

## **Related issues**
Fixes: MetaMask/snaps#2758

## **Manual testing steps**
1. Use custom Snap build from template-snap-monorepo and add few
varieties of address component to test new capabilities.
2. Make sure UI is looking good.
3. Make sure that there are no regressions.

## **Screenshots/Recordings**
### **Before**
![Screenshot 2024-10-11 at 18 12
06](https://github.com/user-attachments/assets/4d498bd6-0ef0-445c-90ab-40918e63b421)

### **After**
![Screenshot 2024-10-15 at 14 34
16](https://github.com/user-attachments/assets/792244b6-300d-45e2-b7e1-91bbcb93a9a6)
![Screenshot 2024-10-15 at 14 34
28](https://github.com/user-attachments/assets/9c6776e1-f6b6-4888-864b-b95621e83069)
![Screenshot 2024-10-15 at 14 34
47](https://github.com/user-attachments/assets/054a9127-ce9a-43d9-ab06-61c4099990a6)
![Screenshot 2024-10-15 at 14 35
10](https://github.com/user-attachments/assets/4c829486-2195-4f14-947e-cbc3572eb822)

### Snap code used for testing
#### (_Might be useful for QA_)
```typescript
export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
  switch (request.method) {
    case 'hello':
      return snap.request({
        method: 'snap_dialog',
        params: {
          type: 'confirmation',
          content: (
            <Box>
              <Heading size="lg">Some unknown address</Heading>

              <Text>Default address</Text>
              <Address address="0x1acC6721CF7642Eb37651F8019AC9a785C064002" />

              <Text>Custom address (displayName=true)</Text>
              <Address
                address="0x1acC6721CF7642Eb37651F8019AC9a785C064002"
                displayName={true}
              />

              <Text>Custom address (avatar=false)</Text>
              <Address
                address="0x1234567890123456789012345678901234567890"
                avatar={false}
              />

              <Text>Custom address (displayName=true, avatar=false)</Text>
              <Address
                address="0x1234567890123456789012345678901234567890"
                displayName={true}
                avatar={false}
              />

              <Text>Custom address (truncate=false)</Text>
              <Address
                address="0x1234567890123456789012345678901234567890"
                truncate={false}
              />

              <Text>Custom address (truncate=false, avatar=false)</Text>
              <Address
                address="0x1234567890123456789012345678901234567890"
                truncate={false}
                avatar={false}
              />

              <Heading size="lg">My address</Heading>

              <Text>Default address</Text>
              <Address address="0xBc641D258aA0e90F70AaDbcC4dE2D58d88029713" />

              <Text>Custom address (displayName=true)</Text>
              <Address
                address="0xBc641D258aA0e90F70AaDbcC4dE2D58d88029713"
                displayName={true}
              />

              <Text>Custom address (avatar=false)</Text>
              <Address
                address="0xBc641D258aA0e90F70AaDbcC4dE2D58d88029713"
                avatar={false}
              />

              <Text>Custom address (displayName=true, avatar=false)</Text>
              <Address
                address="0xBc641D258aA0e90F70AaDbcC4dE2D58d88029713"
                displayName={true}
                avatar={false}
              />

              <Text>Custom address (truncate=false)</Text>
              <Address
                address="0xBc641D258aA0e90F70AaDbcC4dE2D58d88029713"
                truncate={false}
              />

              <Text>Custom address (truncate=false, avatar=false)</Text>
              <Address
                address="0xBc641D258aA0e90F70AaDbcC4dE2D58d88029713"
                truncate={false}
                avatar={false}
              />

              <Heading size="lg">Contact address</Heading>

              <Text>Default address</Text>
              <Address address="0x1acC6721CF7642Eb37651F8019AC9a785C064002" />

              <Text>Custom address (displayName=true)</Text>
              <Address
                address="0x1acC6721CF7642Eb37651F8019AC9a785C064002"
                displayName={true}
              />

              <Text>Custom address (avatar=false)</Text>
              <Address
                address="0x1acC6721CF7642Eb37651F8019AC9a785C064002"
                avatar={false}
              />

              <Text>Custom address (displayName=true, avatar=false)</Text>
              <Address
                address="0x1acC6721CF7642Eb37651F8019AC9a785C064002"
                displayName={true}
                avatar={false}
              />

              <Text>Custom address (truncate=false)</Text>
              <Address
                address="0x1acC6721CF7642Eb37651F8019AC9a785C064002"
                truncate={false}
              />

              <Text>Custom address (truncate=false, avatar=false)</Text>
              <Address
                address="0x1acC6721CF7642Eb37651F8019AC9a785C064002"
                truncate={false}
                avatar={false}
              />
            </Box>
          ),
        },
      });
    default:
      throw new Error('Method not found.');
  }
};
```

## **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/develop/.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/develop/.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.

---------

Co-authored-by: Guillaume Roux <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@david0xd david0xd

Labels
team-snaps
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Add changes to Snaps Address UI component MetaMask/metamask-extension
4 participants
@FrederikBolding @david0xd @eriknson @danroc