fix(settings): remove "Proposed nicknames" from settings search results#40772
Conversation
|
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. |
Builds ready [0e8cf6d]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs
|
📝 Additional Context from Slack Discussion
Root Cause AnalysisThis PR addresses a symptom of an incomplete cleanup from PR #31207, which was intended to remove the
Current Fix ScopeThis PR fixes the user-facing symptom (phantom search result) by removing the Potential Follow-up WorkFor a complete cleanup, consider:
Note on AI-Assisted DevelopmentThis PR was created as an experiment with autonomous agent-driven development workflow:
This demonstrates AI agents can effectively fix user-facing issues, while also highlighting that deeper context about historical decisions and incomplete migrations may be needed for comprehensive fixes. Context captured from Slack discussion by @joao.tavares |
The "externalNameSourcesSetting" entry in the settings search index pointed to a setting that was not consistently visible in the Security tab, causing a phantom search result. Clicking it did nothing. Remove the search entry from SETTINGS_CONSTANTS and the associated settingsRef from the security tab component. The actual toggle UI remains intact for users who have petnamesEnabled. Fixes #40656
…d code Remove the petnamesEnabled preference that was re-introduced after migration 150 due to it remaining in the PreferencesController default state. This adds migration 199 to clean up persisted state, removes the property from the controller type/defaults, deletes dead rendering code in the security-tab (renderExternalNameSourcesToggle and its petnamesEnabled gate), and cleans up all fixtures and test data.
0e8cf6d to
b67f5d3
Compare
Builds ready [b67f5d3]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|
…abled from e2e tests
Builds ready [63acb87]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|
Builds ready [788bc46]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
788bc46 to
cc14bda
Compare
Builds ready [cc14bda]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| ...mockState.metamask, | ||
| preferences: { | ||
| ...mockState.metamask.preferences, | ||
| petnamesEnabled: true, |
There was a problem hiding this comment.
Misleading test name after removing feature flag
Low Severity
The test is named 'renders appropriately with PetNames enabled' but the petnamesEnabled property it used to set has been removed. The test body now just spreads mockState.metamask.preferences into an identical copy with no actual override, making the entire metamask state override a no-op. The test name no longer reflects what it actually verifies, and the redundant state spread adds confusion for future readers.
There was a problem hiding this comment.
That test is still fine.. petnames are now always enabled. We are just removing the property to specify it or not. They are always on. It is part of the wallet
Builds ready [1a734e8]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|
|
Builds ready [aca8484]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|





Description
"Proposed nicknames" (
externalNameSourcesSetting) appeared as a search result in the Settings search, but clicking it did nothing because the corresponding UI toggle is conditionally rendered (gated behindpetnamesEnabled) and the search-to-scroll ref mapping was misaligned.This PR removes the search index entry from
SETTINGS_CONSTANTSso the phantom result no longer appears. The actual toggle UI in the Security tab is left intact for users who havepetnamesEnabledactive.Changelog
CHANGELOG entry: Fixed "Proposed nicknames" appearing as a non-functional result in Settings search
Related issues
Fixes: #40656
Manual testing steps
Screenshots/Recordings
Before
Searching "names" shows "Proposed nicknames" as a search result under Security & privacy — clicking it does nothing:
After
Searching "names" now correctly shows "No matching results found":
Other search terms (e.g. "phishing") continue to work:
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Adds a new state migration (v199) and updates fixtures/tests to match, which can affect persisted user state and E2E reliability if the migration or version bump is wrong. UI changes are limited to settings/search and removing a conditional toggle path.
Overview
Removes the "Proposed nicknames" entry from settings search (
SETTINGS_CONSTANTS) and deletes the corresponding conditional section/toggle wiring from the legacy Security & Privacy tab so the settings search no longer links to a non-functional anchor.Cleans up the deprecated
petnamesEnabledpreference across the app by removing it fromPreferencesControllertypes/defaults, fixtures, and tests, and introducing migration199to deletepreferences.petnamesEnabledfrom persisted state (with fixture migration versions bumped to199).Written by Cursor Bugbot for commit aca8484. This will update automatically on new commits. Configure here.