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

[5.2][Fix] Favourites not showing when home button is pressed in browser tab menu #4175

Merged
merged 14 commits into from
May 13, 2022

Conversation

Fatxx
Copy link
Contributor

@Fatxx Fatxx commented Apr 28, 2022

Description

Fix no favourites shown when "home" is pressed in the browser, scripts are now guaranteed to be present when the website is loaded.

This is blocked by MetaMask/dapps#122 and should be tested and merged together.

How to test

  • Make sure you check out both branches (app and dapp repo);
  • On this line the URL should match your app local URL either http://localhost:3000/ or http://${your-ip}:3000/
  • Also on this line it should be changed accordingly like localhost:3000 or {your-ip}:3000

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

Issue

Progresses #3331

@Fatxx Fatxx requested a review from a team as a code owner April 28, 2022 15:46
@github-actions
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.

@Fatxx Fatxx added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) needs-qa Any New Features that needs a full manual QA prior to being added to a release. type-bug Something isn't working Sev2-normal An issue that may lead to users misunderstanding some limited risks they are taking Mobile QA board labels Apr 28, 2022
Copy link
Member

@andrepimenta andrepimenta left a comment

Choose a reason for hiding this comment

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

We probably need to check if the user is on the homepage before we inject the favourites otherwise we will inject them into all websites. Does it need to be on RPCMethods?

@Fatxx
Copy link
Contributor Author

Fatxx commented Apr 28, 2022

It should only inject on the "metamask dapps web app" since the event is triggered from there.

Maybe not I just used the RpcMethod logic because we were already using it for other events but we can create a custom event for that.

Copy link
Member

@andrepimenta andrepimenta left a comment

Choose a reason for hiding this comment

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

LGTM!

@andrepimenta andrepimenta removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Apr 29, 2022
@Fatxx Fatxx changed the title Fix favourites not showing when home button is pressed in browser tab menu [5.2][Fix] Favourites not showing when home button is pressed in browser tab menu May 10, 2022
@chrisleewilcox chrisleewilcox added QA in Progress QA has started on the feature. and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels May 11, 2022
Copy link
Contributor

@chrisleewilcox chrisleewilcox left a comment

Choose a reason for hiding this comment

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

iOS 15.0 Simulator = Pass
iOS 15.5 iPhone 12 = Pass
Android 10 Pixel 3 Simulator = Pass
Android 12 Samsung A42 = Pass

Browser Favorites
Tap Home button shows Favorites = Pass
New Browser Tab show Favorites = Pass
Delete Favorites = Pass

@chrisleewilcox chrisleewilcox added QA Passed A successful QA run through has been done and removed QA in Progress QA has started on the feature. labels May 12, 2022
@Fatxx Fatxx merged commit 83e7f52 into main May 13, 2022
@Fatxx Fatxx deleted the fix/home-favorites branch May 13, 2022 10:14
@github-actions github-actions bot locked and limited conversation to collaborators May 13, 2022
@mobularay mobularay added the release-5.2.0 Issues/PRs confirmed for prod release 5.2.0 label May 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
QA Passed A successful QA run through has been done release-5.2.0 Issues/PRs confirmed for prod release 5.2.0 Sev2-normal An issue that may lead to users misunderstanding some limited risks they are taking type-bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants