-
Notifications
You must be signed in to change notification settings - Fork 18
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
Fix/ens name #422
base: develop
Are you sure you want to change the base?
Fix/ens name #422
Conversation
@heronlancellot is attempting to deploy a commit to the Blockful Team on Vercel. A member of the Team first needs to authorize it. |
components/01-atoms/ENSAvatar.tsx
Outdated
import { EthereumAddress } from "@/lib/shared/types"; | ||
import cc from "classcat"; | ||
import { useEffect, useState } from "react"; | ||
// /* eslint-disable react-hooks/exhaustive-deps */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whenever commenting out features from the project, can you please add a documentation at the top of the comments explaining this feature is out for a while?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I'd rather using this comment methodology /** */
components/01-atoms/SearchBar.tsx
Outdated
import { SwapContext } from "@/lib/client/contexts"; | ||
import { useContext, useEffect } from "react"; | ||
import { ENS } from "web3-eth-ens"; | ||
import Web3 from "web3"; | ||
// import { ENS } from "web3-eth-ens"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing around comment methodology in here
const { primaryName } = useEnsData({ | ||
ensAddress: authenticatedUserAddress, | ||
}); | ||
// const { primaryName } = useEnsData({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here!
}); | ||
// const { primaryName } = useEnsData({ | ||
// ensAddress: authenticatedUserAddress, | ||
// }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add docs above the comments (one per file is enough) saying why this feature is out for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For each file, please
searchedENSName | ||
? `${searchedENSName} offers` | ||
: validatedAddressToSwap | ||
// searchedENSName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the other comment syntax
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
> | ||
<ENSAvatar avatarENSAddress={authenticatedUserAddress} /> | ||
{/* <ENSAvatar avatarENSAddress={authenticatedUserAddress} /> */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here as well, adding a comment on why this is out
@@ -64,15 +64,16 @@ export const UserOfferInfo = ({ | |||
<div> | |||
<div className="flex gap-2"> | |||
<div> | |||
{address && ( | |||
{/* {address && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here
@@ -7,7 +7,7 @@ import { | |||
|
|||
export const SwapSection = () => { | |||
return ( | |||
<div className="max-w-[1280px] xl:max-h-[720px] w-full flex xl:flex-row flex-col lg:justify-center"> | |||
<div className="max-w-[1280px] xl:max-h-[720px] w-full flex xl:flex-row flex-col lg:justify-center xl:h-full"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already in another PR, shall update with merge
lib/client/hooks/useENSData.tsx
Outdated
SUCCESS, | ||
ERROR, | ||
} | ||
// export enum ENSAvatarQueryStatus { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please refer to the above comments and apply the mentioned rules here!
components/01-atoms/index.ts
Outdated
@@ -3,7 +3,7 @@ export * from "./ApproveTokenCards"; | |||
export * from "./BlockExplorerExternalLinkButton"; | |||
export * from "./ConnectWallet"; | |||
export * from "./DisconnectWallet"; | |||
export * from "./ENSAvatar"; | |||
// export * from "./ENSAvatar"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment in here as well
lib/client/hooks/useENSData.tsx
Outdated
@@ -1,3 +1,23 @@ | |||
/** | |||
* @deprecated This hook is deprecated because the searched ENS address | |||
* sometimes returns incorrect results, which negatively impacts the user flow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replicate my suggestions above to the exact same text
lib/client/hooks/useENSData.tsx
Outdated
* `ensAddress` changes. It updates the state variables `primaryName` and `avatarQueryStatus` | ||
* based on the fetched data or any errors encountered during the fetch process. | ||
* | ||
* @param {Props} props - The properties object. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not type the whole props object but each single prop
lib/client/hooks/useENSData.tsx
Outdated
* based on the fetched data or any errors encountered during the fetch process. | ||
* | ||
* @param {Props} props - The properties object. | ||
* @param {EthereumAddress | null} props.ensAddress - The ENS address to fetch the data for. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not mention props.
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
<div className="flex flex-col"> | ||
<div className="flex items-center justify-start gap-2"> | ||
{primaryName && ( | ||
{/* {primaryName && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment section can reproduce the method of commenting deprecated feature that the other sections received! Please go ahead and do so 🧑🏼💻👊🏼🧑🏼💻
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
const { primaryName } = useEnsData({ | ||
ensAddress: authenticatedUserAddress, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is missing documentation on why it is commented out
Co-authored-by: caco.eth <[email protected]>
Co-authored-by: caco.eth <[email protected]>
Closes #320