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

fix: add logo support for PYUSD & USDP #347

Merged
merged 6 commits into from
Aug 28, 2024
Merged
Changes from 5 commits
Commits
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
20 changes: 19 additions & 1 deletion app/address/[address]/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,11 @@ function AccountHeader({
tokenInfo?: FullTokenInfo;
isTokenInfoLoading: boolean;
}) {
interface State {
name: string;
image: string;
}
const [tokenData, setTokenData] = React.useState<State>({ image: '', name: '' });
const mintInfo = useMintAccountInfo(address);

const parsedData = account?.data.parsed;
Expand Down Expand Up @@ -286,10 +291,23 @@ function AccountHeader({
const tokenMetadata = create(metadataExtension.state, TokenMetadata);
Copy link
Contributor Author

@catalinred catalinred Jun 12, 2024

Choose a reason for hiding this comment

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

@jnwng I tried to implement your code suggestion but unfortunately didn't succeed. It seems that tokenMetadata is available only within this conditional, which is why I added the IIFE function within AccountHeader.

From my testing, I haven't noticed any inconsistencies with the state, but please let me know if you observe otherwise.

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm okay, gotcha. i think this is okay for now

can you do some validation on the response / some error handling?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jnwng I just added a basic error handling block.

const { metadataAddress } = create(metadataPointerExtension.state, MetadataPointer);

(async () => {
try {
// Avoid re-renders once tokenData has been set
if (!tokenData.name && !tokenData.image) {
const response = await fetch(tokenMetadata.uri);
setTokenData(await response.json());
Copy link
Collaborator

Choose a reason for hiding this comment

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

since we don't trust this URI, probably makes sense to unpack the response and only take the fields we want

Copy link
Collaborator

Choose a reason for hiding this comment

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

This layout is called in every single page load of the Explorer. If there's an error fetching the URI it will brick loading the rest of the page.

Better way to do this is to refactor all of this in the same pattern as the <CompressedNftAccountHeader/> component. Once we're inside this IIFE block, then return a T22AccountHeader component.

In the T22AccountHeader component, instead of executing a naked fetch, try using useMetadataJsonLink from @/app/providers/compressed-nft.

}
} catch (err) {
console.error(err);
}
})();

// Handles the basic case where MetadataPointer is reference the Token Metadata extension directly
// Does not handle the case where MetadataPointer is pointing at a separate account.
if (metadataAddress?.toString() === address) {
token.name = tokenMetadata.name;
token.name = tokenData?.name;
token.logoURI = tokenData?.image;
}
}
// Fall back to legacy token list when there is stub metadata (blank uri), updatable by default by the mint authority
Expand Down
Loading