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

Show contract names in petnames by default #22735

Closed
dbrans opened this issue Jan 30, 2024 · 0 comments · Fixed by #22734
Closed

Show contract names in petnames by default #22735

dbrans opened this issue Jan 30, 2024 · 0 comments · Fixed by #22734
Assignees
Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. release-11.11.0 Issue or pull request that will be included in release 11.11.0

Comments

@dbrans
Copy link
Contributor

dbrans commented Jan 30, 2024

Objective:

To develop and integrate a new useDisplayName hook into the component. This hook aims to replicate the existing displayName logic, specifically in the context of the display of token names.

Development of useDisplayName Hook:

Create a new hook, useDisplayName, encapsulating the logic currently used to determine displayName.
The hook should sequentially check for:

  • Petname (if available).
  • Token metadata (from either static or dynamic token lists).
  • ENS Entry

In the event that no name is found for the address, a fallback will be used: a shortened version of the address.

Ensure the hook returns a string value representing the most appropriate display name based on the above checks.

Integration with <Name/> Component:

  • Replace the current useName logic in the <Name/> component with the newly created useDisplayName hook.
  • Ensure we show the correct icon (bookmark vs warning) based on whether or not a name was found.
@dbrans dbrans self-assigned this Jan 30, 2024
@metamaskbot metamaskbot added the INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. label Jan 30, 2024
dbrans added a commit that referenced this issue Jan 31, 2024
## **Description**

This PR introduces a new `useDisplayName` hook, aimed at including logic
for displaying token names within the `<Name/>` component. This hook
replicates existing display logic, before Petnames.

## Details

### Development of `useDisplayName` Hook

- **Functionality**: The new `useDisplayName` hook encapsulates the
existing logic for determining a token's displayName. It performs
sequential checks for:
  - **Petname**: Prioritizes the petname if available.
- **Token Metadata**: Fetches the name from static or dynamic token
lists.

### Integration with `<Name/>` Component

- **Replacement of Logic**: The existing `useName` logic within the
`<Name/>` component is replaced by the `useDisplayName` hook.

- **Icon Display**: The component adjusts the displayed icon (either a
bookmark or a warning sign) based on the presence or absence of a
resolved name.

## **Related issues**

Fixes: #22735

## **Manual testing steps**

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<img width="237" alt="image"
src="https://github.com/MetaMask/metamask-extension/assets/507015/50bf8929-ca12-41c2-a84c-f689e853d4cd">


### **After**

<img width="238" alt="image"
src="https://github.com/MetaMask/metamask-extension/assets/507015/49c08b74-9c35-4b73-b318-9053f71fd375">

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained what problem this PR is solving and how it
is solved.
- [ ] I've linked related issues
- [ ] I've included manual testing steps
- [ ] I've included screenshots/recordings if applicable
- [ ] 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.
- [ ] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
@metamaskbot metamaskbot added the release-11.11.0 Issue or pull request that will be included in release 11.11.0 label Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. release-11.11.0 Issue or pull request that will be included in release 11.11.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants