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

Update React Tutorial (remove detect-metamask) #1299

Merged
merged 19 commits into from
May 25, 2024

Conversation

httpJunkie
Copy link
Contributor

@httpJunkie httpJunkie commented May 6, 2024

Description

We want to revamp the Tutorials #1 and #2, this PR is for the update of the first of those two tutorials Create a React dapp with local state. And the intention is to keep the same idea for bnoth tutorials with updates to the code and explanation using EIP-6963 instead of window.ethereum and @metamask/detect-provider.

I may add some links out to existing pages that speak on EIP-6963 from the tutorial rather than covering the topic as we already have docs "Wallet Interoperability" and "Detecting MetaMask" that we could create cross links to.

Issue(s) fixed

Tutorial instruction and code have been update, I still need to link to an updated GitHub Repo so that when this goes live the prior repo is not affected until then.

Fixes #

Preview

Screen.Recording.2024-05-17.at.5.47.05.PM.mov

Checklist

Complete this checklist before merging your PR:

  • If this PR contains a major change to the documentation content, I have added an entry to the top of the "What's new?" page.
  • The proposed changes have been reviewed and approved by a member of the documentation team.

@httpJunkie httpJunkie requested review from a team as code owners May 6, 2024 15:56
Copy link

github-actions bot commented May 6, 2024

Preview published: update-react-tutorials-86-mm-detect

@alexandratran alexandratran self-assigned this May 6, 2024
Copy link
Contributor

@alexandratran alexandratran left a comment

Choose a reason for hiding this comment

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

Thanks for this update! A few initial comments:

  1. I think we can get rid of step 2. Discover Wallet Providers (Browser Extensions). The beginner tutorial (Update beginner's tutorial  #1276) should cover this.
  2. Can we split the current step 3. Detect Multiple Wallet Providers into multiple steps or multiple sub-steps so it's not so long? Maybe setting up the environment variables, creating the hooks, and adding the buttons and CSS. If this makes sense to you I can go ahead and add those edits.
  3. Can you add screenshots to this page demonstrating what the user might see? We need to update the screenshot at the end of step 1, and add another at the end of the tutorial.

Copy link

github-actions bot commented May 7, 2024

Preview published: update-react-tutorials-86-mm-detect

Copy link

github-actions bot commented May 7, 2024

Preview published: update-react-tutorials-86-mm-detect

Copy link

github-actions bot commented May 7, 2024

Preview published: update-react-tutorials-86-mm-detect

Copy link

github-actions bot commented May 7, 2024

Preview published: update-react-tutorials-86-mm-detect

Copy link

github-actions bot commented May 7, 2024

Preview published: update-react-tutorials-86-mm-detect

Copy link

Preview published: update-react-tutorials-86-mm-detect

Copy link

Preview published: update-react-tutorials-86-mm-detect

@httpJunkie
Copy link
Contributor Author

httpJunkie commented May 15, 2024

This "React with local state" above is the final draft from @httpJunkie (Eric Bishard).
We need to approve and wait and send the "React with global state tutorial" out at the same time together.

But we can still get this review approved and out of the way.

Copy link

Preview published: update-react-tutorials-86-mm-detect

Copy link

Preview published: update-react-tutorials-86-mm-detect

Copy link
Contributor

@alexandratran alexandratran left a comment

Choose a reason for hiding this comment

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

Edited the content for clarity and to create more digestible steps. One question about the screenshots provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

The two images provided are the same. Should this instead be a screenshot of the final dapp which displays an address for the connected wallet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I may have used the same image twice, was not the intention, let me look at it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thats fixed, also I see how you did the CSS and it's funny. That's how I had originally had the article formatted and then I changed something and got lazy. lol It's perfect now. Thank you @alexandratran

Copy link

Preview published: update-react-tutorials-86-mm-detect

Copy link

Preview published: update-react-tutorials-86-mm-detect

Copy link
Contributor

@alexandratran alexandratran left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

Preview published: update-react-tutorials-86-mm-detect

Copy link

Preview published: update-react-tutorials-86-mm-detect

@httpJunkie httpJunkie merged commit 300340d into main May 25, 2024
8 checks passed
@httpJunkie httpJunkie deleted the update-react-tutorials-86-mm-detect branch May 25, 2024 06:20
@alexandratran alexandratran mentioned this pull request May 31, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants