Skip to content

Conversation

@ziad-saab
Copy link
Contributor

@ziad-saab ziad-saab commented Jun 3, 2025

Description

This PR changes the behaviour of the "Discover" tab (4th button in the bottom bar) in the following ways:

  1. Most important, every time the discover button is pressed, the user is taken to the popular tokens screen
  2. Contrary to a regular browser tab, the popular tokens screen doesn't have a bottom bar. instead it has a single button next to the URL bar, which either opens a new browser tab when there are no tabs, or opens the list of tabs otherwise
  3. From the discovery screen, the user can explore tokens
  4. From the discovery screen, the user can focus on the URL bar
  5. When the URL bar is focused, it shows a list of recently visited sites (soon tokens too), and bookmarks. The user can use these to navigate, or type a URL and press enter to navigate to it
  6. When the user is looking at a regular browser tab, they can get back to the discovery screen by pressing either the Home button in the bottom bar, or the discover button in the tab bar.
  7. When all tabs are closed, the user goes back to the discovery screen.

Three smaller PRs have been merged into this one after being reviewed and approved by @MarioAslau :

Related issues

Fixes: MMPD-1590

Manual testing steps

  1. Go to the browser
  2. Notice that the popular tokens screen gets loaded
  3. Press on a token name, notice navigation to the token details page
  4. Press on the swap button next to a token, notice navigation to the swaps screen
  5. Enter a URL in the search bar, press ENTER and notice that a new tab opens with that URL
  6. Press the tabs button, then press "Close All", notice that you get back to the discovery screen
  7. With all tabs closed, notice that the discovery screen's tab button shows a + instead of a number of tabs
  8. Press the +, notice a new tab opens with portfolio
  9. Press the discover button, notice you get back to the discovery screen

Screenshots/Recordings

Before

Simulator.Screen.Recording.-.iPhone.16.Plus.-.2025-05-28.at.17.57.16.mp4

After

discovery.mp4

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 addrex sses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@ziad-saab ziad-saab added QA Passed QA testing has been completed and passed team-portfolio Run Smoke E2E labels Jun 3, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2025

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.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2025

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: e88102b
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/11169320-43d6-4fe5-9928-af12eb23579c

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@socket-security
Copy link

socket-security bot commented Jun 3, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatednpm/​@​metamask/​token-search-discovery-controller@​3.1.0 ⏵ 3.3.0871007295 +3100

View full report

@ziad-saab ziad-saab marked this pull request as ready for review June 10, 2025 21:08
@ziad-saab ziad-saab requested a review from a team June 10, 2025 21:08
@github-actions
Copy link
Contributor

github-actions bot commented Jun 10, 2025

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 40a2f30
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/cec0f326-743a-472a-8fb7-0738f9954b69

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

brianacnguyen
brianacnguyen previously approved these changes Jun 26, 2025
@github-actions
Copy link
Contributor

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: f2070d0
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/267934a5-fe5f-4b6a-93f6-81e6a57452e9

Note

  • This comment will auto-update when build completes
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@sonarqubecloud
Copy link

@github-actions
Copy link
Contributor

github-actions bot commented Jun 26, 2025

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: b3b96be
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/58a7e068-7063-42de-a1b6-42c3d1a77450

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

Copy link
Contributor

@MarioAslau MarioAslau left a comment

Choose a reason for hiding this comment

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

LGTM

@ziad-saab
Copy link
Contributor Author

@MetaMask/swaps-engineers would you please take a look at this PR? I think it's tagging you because there are changes to useSwapBridgeNavigation. Please reach out on Slack if you have any questions.

// The await above isn't enough for the network to actually be set
await waitForCondition({
fn: () => selectChainId(store.getState()) === swapToken.chainId,
timeout: 2000,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the 2000 timeout value guaranteed to provide deterministic results, or is it something that might need to be adjusted again in the future? Also, how did we arrive at this specific number?

Copy link
Contributor

@infiniteflower infiniteflower Jun 30, 2025

Choose a reason for hiding this comment

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

What happens if this line is removed? Because we've been using this code without this waitForCondition before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external-contributor QA Passed QA testing has been completed and passed team-portfolio

Projects

Status: Needs dev review

Development

Successfully merging this pull request may close these issues.

10 participants