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

Resolve issue with the MetaMask button #1086

Merged

Conversation

0xlukem
Copy link
Contributor

@0xlukem 0xlukem commented Jan 24, 2025

Description

I added a missing event listener for the .connectMetaMask button in the body so it calls the same connectNetwork function used by the nav button. This ensures that the body button now correctly connects to the specified network.

Checklist

  • I have added a label to this PR 🏷️
  • I have run my changes through Grammarly
  • If pages have been moved around, I have created an additional PR in moonbeam-mkdocs to update redirects

@themacexpert themacexpert self-requested a review January 24, 2025 18:15
themacexpert
themacexpert previously approved these changes Jan 24, 2025
Copy link
Contributor

@themacexpert themacexpert left a comment

Choose a reason for hiding this comment

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

I had to clear cookies but after that it worked fine! LGTM

@themacexpert themacexpert added the B1 - Ready to be Merged Pull request is ready to be merged label Jan 24, 2025
Copy link
Contributor

@eshaben eshaben left a comment

Choose a reason for hiding this comment

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

Sweeeet! It's working now!

On the Connect MetaMask page, after you connect to a network, the button text doesn't change to show "Connected <shortened_account_here>" and disables the button like it does in the modal. So ideally, that should be added in because if I am already connected to a network, but I try to click the button again, nothing happens. I don't think anyone would do this haha, but you never know. Anyways, so if the button was disabled, this issue would be avoided.

js/connectMetaMask.js Outdated Show resolved Hide resolved
js/connectMetaMask.js Outdated Show resolved Hide resolved
js/connectMetaMask.js Outdated Show resolved Hide resolved
};

// Original handleError
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

js/connectMetaMask.js Outdated Show resolved Hide resolved
js/connectMetaMask.js Outdated Show resolved Hide resolved
@eshaben eshaben added A2 - Maintenance Minor Pull request contains minor updates to an existing page (i.e., modifying parameters, steps, etc.) C1 - Medium Medium priority task and removed B1 - Ready to be Merged Pull request is ready to be merged labels Jan 28, 2025
@0xlukem 0xlukem requested a review from eshaben January 28, 2025 13:46
Copy link
Contributor

@eshaben eshaben left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@eshaben eshaben merged commit e5947b0 into moonbeam-foundation:master Jan 28, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A2 - Maintenance Minor Pull request contains minor updates to an existing page (i.e., modifying parameters, steps, etc.) C1 - Medium Medium priority task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants