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: Add fuzzy matching for name lookup #25264

Merged
merged 8 commits into from
Jul 15, 2024

Conversation

hmalik88
Copy link
Contributor

@hmalik88 hmalik88 commented Jun 12, 2024

Description

  1. What is the reason for the change? Snaps can't indicate whether they've exact matched or fuzzy matched without responding with a domainName property that the UI can display.
  2. What is the improvement/solution? Changes in the namelookup API are being integrated and the UI logic was changed to display the domainName returned in the snap response instead of userInput.

Demo:

Screen.Recording.2024-06-20.at.8.49.16.AM.mov

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.

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 metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jun 12, 2024
@hmalik88 hmalik88 added area-snaps team-snaps-platform Snaps Platform team and removed INVALID-PR-TEMPLATE PR's body doesn't match template labels Jun 12, 2024
@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jun 12, 2024
Copy link

codecov bot commented Jun 12, 2024

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 64.92%. Comparing base (aabd481) to head (394770d).

Current head 394770d differs from pull request most recent head 8f50e1d

Please upload reports for the commit 8f50e1d to get more accurate results.

Files Patch % Lines
...nts/multichain/pages/send/components/recipient.tsx 0.00% 3 Missing ⚠️
ui/ducks/domains.js 0.00% 1 Missing ⚠️
...nd-content/add-recipient/domain-input.component.js 0.00% 1 Missing ⚠️
...tact-list-tab/add-contact/add-contact.component.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #25264      +/-   ##
===========================================
- Coverage    64.93%   64.92%   -0.00%     
===========================================
  Files         1385     1385              
  Lines        54870    54874       +4     
  Branches     14408    14408              
===========================================
  Hits         35625    35625              
- Misses       19245    19249       +4     

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

@hmalik88 hmalik88 removed the INVALID-PR-TEMPLATE PR's body doesn't match template label Jun 20, 2024
@hmalik88 hmalik88 marked this pull request as ready for review June 20, 2024 12:54
@hmalik88 hmalik88 requested review from a team as code owners June 20, 2024 12:54
@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jun 20, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [394770d]
Page Load Metrics (114 ± 149 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint5615176199
domContentLoaded8161021
load381467114311149
domInteractive8161021
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 24 Bytes (0.00%)
  • common: 13 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [c645fc4]
Page Load Metrics (228 ± 244 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint7213893178
domContentLoaded9291353
load411837228509244
domInteractive9291363
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 24 Bytes (0.00%)
  • common: 13 Bytes (0.00%)

Copy link

Copy link
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed files owned by confirmation team, changes look good 👍

@metamaskbot
Copy link
Collaborator

Builds ready [859d098]
Page Load Metrics (60 ± 6 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint7112090116
domContentLoaded95627147
load398660126
domInteractive95627147
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 24 Bytes (0.00%)
  • common: 13 Bytes (0.00%)

@hmalik88 hmalik88 merged commit 7b3450a into develop Jul 15, 2024
78 of 79 checks passed
@hmalik88 hmalik88 deleted the hm/domain-resolution-fuzzy-matching branch July 15, 2024 15:26
@github-actions github-actions bot locked and limited conversation to collaborators Jul 15, 2024
@metamaskbot metamaskbot added the release-12.3.0 Issue or pull request that will be included in release 12.3.0 label Jul 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-snaps INVALID-PR-TEMPLATE PR's body doesn't match template release-12.3.0 Issue or pull request that will be included in release 12.3.0 team-snaps-platform Snaps Platform team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants