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.

Sign up for GitHub

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

[Gateway] - FE Fixes for non-MM users #1690

Merged
merged 4 commits into from
Dec 11, 2023
Merged

[Gateway] - FE Fixes for non-MM users #1690

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a02cb4c
feat:
Jennievon Dec 11, 2023
903d2a0
Merge branch 'main' of https://github.com/ten-protocol/go-ten into je…
Jennievon Dec 11, 2023
318d088
fix: serve files in dist directory in `.next` for dev
Jennievon Dec 11, 2023
d042e6f
add comment for config
Jennievon Dec 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions tools/walletextension/frontend/src/api/ethRequests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const typedData = {

export const switchToTenNetwork = async () => {
if (!ethereum) {
throw new Error("No ethereum object found");
throw "No ethereum object found";
Copy link

Choose a reason for hiding this comment

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

The change from throwing an Error object to a string literal may impact error handling logic downstream that relies on properties of Error objects. Consider throwing a new Error with a message instead to maintain consistency with error object properties.

- throw "No ethereum object found";
+ throw new Error("No ethereum object found");
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
throw "No ethereum object found";
throw new Error("No ethereum object found");

}
try {
await ethereum.request({
Expand All @@ -55,7 +55,7 @@ export const switchToTenNetwork = async () => {

export const connectAccounts = async () => {
if (!ethereum) {
throw new Error("No ethereum object found");
throw "No ethereum object found";
Copy link

Choose a reason for hiding this comment

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

This comment is the same as the previous one. Ensure that the error handling logic is consistent across the application by throwing Error objects instead of string literals.

- throw "No ethereum object found";
+ throw new Error("No ethereum object found");
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
throw "No ethereum object found";
throw new Error("No ethereum object found");

}
try {
return await ethereum.request({
Expand All @@ -69,7 +69,7 @@ export const connectAccounts = async () => {

export const getSignature = async (account: string, data: any) => {
if (!ethereum) {
throw new Error("No ethereum object found");
throw "No ethereum object found";
Copy link

Choose a reason for hiding this comment

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

As with the previous instances, throwing a string literal here can lead to issues with error handling that expects an Error object.

- throw "No ethereum object found";
+ throw new Error("No ethereum object found");
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
throw "No ethereum object found";
throw new Error("No ethereum object found");

}
try {
return await ethereum.request({
Expand All @@ -78,7 +78,7 @@ export const getSignature = async (account: string, data: any) => {
});
} catch (error) {
console.error(error);
throw new Error("Failed to get signature");
throw "Failed to get signature";
Copy link

Choose a reason for hiding this comment

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

The thrown string literal here should be replaced with an Error object to ensure that any error handling expecting an Error object can function correctly.

- throw "Failed to get signature";
+ throw new Error("Failed to get signature");
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
throw "Failed to get signature";
throw new Error("Failed to get signature");

}
};

Expand Down Expand Up @@ -110,7 +110,7 @@ export const getToken = async (provider: ethers.providers.Web3Provider) => {

export async function addNetworkToMetaMask(rpcUrls: string[]) {
if (!ethereum) {
throw new Error("No ethereum object found");
throw "No ethereum object found";
Copy link

Choose a reason for hiding this comment

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

Again, it's important to throw Error objects to maintain consistency in error handling throughout the application.

- throw "No ethereum object found";
+ throw new Error("No ethereum object found");
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
throw "No ethereum object found";
throw new Error("No ethereum object found");

}
try {
await ethereum.request({
Expand Down
12 changes: 10 additions & 2 deletions tools/walletextension/frontend/src/components/modules/common/connect-wallet.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Button } from "../../ui/button";
import useGatewayService from "../../../services/useGatewayService";
import { Link2Icon, LinkBreak2Icon } from "@radix-ui/react-icons";
import React from "react";
import { downloadMetaMask, ethereum } from "@/lib/utils";
const ConnectWalletButton = () => {
const { walletConnected, revokeAccounts } = useWalletConnection();
const { connectToTenTestnet } = useGatewayService();
Expand All @@ -11,7 +12,14 @@ const ConnectWalletButton = () => {
<Button
className="text-sm font-medium leading-none"
variant={"outline"}
onClick={walletConnected ? revokeAccounts : connectToTenTestnet}
onClick={
ethereum
? walletConnected
? revokeAccounts
: connectToTenTestnet
: downloadMetaMask
}
suppressHydrationWarning
>
{walletConnected ? (
<>
Expand All @@ -21,7 +29,7 @@ const ConnectWalletButton = () => {
) : (
<>
<Link2Icon className="h-4 w-4 mr-1" />
Connect
{ethereum ? "Connect" : "Install"}
</>
)}
</Button>
Expand Down
11 changes: 9 additions & 2 deletions tools/walletextension/frontend/src/components/modules/home/disconnected.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
} from "../../ui/dialog";
import Copy from "../common/copy";
import { testnetUrls, tenChainIDDecimal } from "../../../lib/constants";
import { downloadMetaMask, ethereum } from "@/lib/utils";

const CONNECTION_STEPS = [
"Hit Connect to Ten and start your journey",
Expand All @@ -25,6 +26,7 @@ const CONNECTION_STEPS = [

const Disconnected = () => {
const { connectToTenTestnet } = useGatewayService();

return (
<div className="flex flex-col items-center justify-center space-y-4">
<h1 className="text-4xl font-bold">Welcome to the Ten Gateway!</h1>
Expand Down Expand Up @@ -102,9 +104,14 @@ const Disconnected = () => {
</DialogContent>
</Dialog>

<Button className="mt-4" onClick={connectToTenTestnet}>
<Button
className="mt-4"
onClick={ethereum ? connectToTenTestnet : downloadMetaMask}
>
<Terminal />
<span className="ml-2">Connect to Ten Testnet</span>
<span className="ml-2">
{ethereum ? "Connect to Ten Testnet" : "Install MetaMask to continue"}
</span>
</Button>
</div>
);
Expand Down
7 changes: 7 additions & 0 deletions tools/walletextension/frontend/src/lib/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ export function getRPCFromUrl() {
}

export async function isTenChain() {
if (!ethereum) {
return false;
}
let currentChain = await ethereum.request({
method: "eth_chainId",
});
Expand All @@ -64,3 +67,7 @@ export async function isTenChain() {

export const { ethereum } =
typeof window !== "undefined" ? window : ({} as any);

export const downloadMetaMask = () => {
window ? window.open("https://metamask.io/download", "_blank") : null;
};
3 changes: 2 additions & 1 deletion tools/walletextension/frontend/src/services/ethService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const ethService = {
if (ethereum && ethereum.isMetaMask) {
return;
} else {
showToast(
return showToast(
ToastType.WARNING,
"Please install MetaMask to use Ten Gateway."
);
Expand Down Expand Up @@ -96,6 +96,7 @@ const ethService = {
})
);
updatedAccounts = await Promise.all(authenticationPromise);
showToast(ToastType.SUCCESS, "Account authentication status updated!");
return updatedAccounts;
},

Expand Down
2 changes: 1 addition & 1 deletion tools/walletextension/frontend/src/services/useGatewayService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const useGatewayService = () => {

await fetchUserAccounts();
} catch (error: any) {
showToast(ToastType.DESTRUCTIVE, `${error.message}`);
showToast(ToastType.DESTRUCTIVE, `${error}`);
throw error;
Comment on lines 60 to 62
Copy link

Choose a reason for hiding this comment

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

The showToast function is being passed the entire error object, which may not result in a user-friendly message. Consider using error.message to ensure that the toast displays a meaningful error message to the user.

- showToast(ToastType.DESTRUCTIVE, `${error}`);
+ showToast(ToastType.DESTRUCTIVE, error.message || 'An unknown error occurred');
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
} catch (error: any) {
showToast(ToastType.DESTRUCTIVE, `${error.message}`);
showToast(ToastType.DESTRUCTIVE, `${error}`);
throw error;
} catch (error: any) {
showToast(ToastType.DESTRUCTIVE, error.message || 'An unknown error occurred');
throw error;

} finally {
setLoading(false);
Expand Down
Loading