feat/improve T22 account page#534
Merged
ngundotra merged 5 commits intosolana-foundation:masterfrom Apr 10, 2025
Merged
Conversation
|
@rogaldh is attempting to deploy a commit to the Solana Foundation Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to f41dd7e in 2 minutes and 58 seconds
More details
- Looked at
179lines of code in4files - Skipped
0files when reviewing. - Skipped posting
10drafted comments based on config settings.
1. app/components/account/TokenExtensionsCard.tsx:36
- Draft comment:
Typo: "satisty" should be "satisfy". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While the typo correction is technically accurate, it's in a comment rather than in code. Comments are documentation and typos in them don't affect functionality. The rules state we should only keep comments that require clear code changes and not make purely informative comments. A typo in a comment is very minor.
The typo could make the code slightly less professional looking and might momentarily confuse readers. Documentation quality does matter.
While documentation quality matters, this is an extremely minor issue that doesn't impact code understanding or functionality. The meaning is still clear despite the typo.
Delete this comment as it's too minor and doesn't require a functional code change.
2. app/components/account/TokenExtensionsCard.tsx:56
- Draft comment:
Consider memoizing the result of populateTokenExtensions if the mintExtensions prop rarely changes. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
3. app/components/account/TokenAccountSection.tsx:512
- Draft comment:
Ensure the removal of TokenExtensionRow mapping is intentional, as this change affects how token extensions are displayed in the account section. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
4. app/components/account/TokenExtensionsCard.tsx:59
- Draft comment:
Using a Map to accumulate extensions keyed by the extension type deduplicates entries. Confirm that losing multiple entries of the same type is intended. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. app/components/account/TokenExtensionsCard.tsx:95
- Draft comment:
Unreachable 'break' statements after return in populatePartialParsedTokenExtension can be removed to clean up the code. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. app/components/account/TokenExtensionsCard.tsx:37
- Draft comment:
Throwing an error during render (when tokenInfo is missing or decimals is null) relies on an external ErrorBoundary. Consider returning a fallback UI or adding more context for better debugging. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
7. app/address/[address]/token-extensions/page-client.tsx:18
- Draft comment:
Consider changing 'Can not fetch extensions.' to 'Cannot fetch extensions.' for a more standard contraction. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
The comment is about UI text content. According to the rules, we should "NOT comment on any pure frontend UI or styling changes, only logic. Assume that if the author changed the UI, they did it correctly." While "Cannot" is technically more standard, this is purely a UI text change with no impact on functionality.
The change could improve text consistency across the codebase. Maybe there are style guidelines that require "cannot" over "can not".
Even if there are style guidelines, UI text changes should be handled through a different process, not PR comments. The rules explicitly say to ignore UI changes.
Delete this comment as it relates purely to UI text content, which we are instructed to ignore.
8. app/components/account/TokenAccountSection.tsx:281
- Draft comment:
In the Bridged Asset Contract row, the tag's href is using 'tokenInfo.extensions.bridgeContract' instead of 'tokenInfo.extensions.assetContract'. Please update it to use the correct asset contract URL. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. app/components/account/TokenAccountSection.tsx:980
- Draft comment:
In the mapping over 'extension.additionalMetadata', the key is provided as a string literal "{keyValuePair[0]}". It should be written as {keyValuePair[0]} without extra quotes so that the key is correctly evaluated. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. app/components/account/TokenExtensionsCard.tsx:36
- Draft comment:
Typographical error: Replace 'satisty' with 'satisfy' in the comment on checking decimals. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While the comment is technically correct about the typo, fixing typos in comments is a very minor issue that doesn't affect functionality. The rules state not to make comments that are obvious or unimportant. A typo in a comment is quite unimportant.
The typo could be seen as reducing code quality and professionalism. Poor spelling in comments could make the code less maintainable.
While code quality is important, this is an extremely minor issue that doesn't warrant a PR comment. The rules explicitly state not to make obvious or unimportant comments.
The comment should be deleted as it points out an unimportant typo that doesn't meaningfully impact code quality.
Workflow ID: wflow_U8BCmRpHFUboTTvT
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Contributor
|
CI not passing because forks can't access secrets / vars |
7 tasks
ngundotra
pushed a commit
that referenced
this pull request
Apr 17, 2025
## Description This PR adds clickable bades with extensions to T22 page. ## Type of change - [x] New feature ## Screenshots https://github.com/user-attachments/assets/79c8c23d-868f-4b4e-9b2a-83b1125bb6f1 ## Testing `pnpm t` ## Related Issues #534 ## Checklist - [x] My code follows the project's style guidelines - [x] I have added tests that prove my fix/feature works - [x] All tests pass locally and in CI - [x] I have updated documentation as needed - [x] CI/CD checks pass - [x] I have included screenshots for protocol screens (if applicable) <!-- ELLIPSIS_HIDDEN --> ---- > [!IMPORTANT] > Add clickable badges for token extensions on T22 page with Storybook support and tests. > > - **Features**: > - Add clickable badges for token extensions on T22 page in `TokenExtensionsStatusRow.tsx` and `TokenExtensionsSection.tsx`. > - Implement `TokenExtensionBadge` and `TokenExtensionBadges` components for displaying extension statuses. > - Introduce `useTokenExtensionNavigation` hook for managing navigation to token extensions. > - **Storybook**: > - Add Storybook configuration in `.storybook/main.ts` and `.storybook/preview.tsx`. > - Add stories for `TokenExtensionBadge` and `TokenExtensionBadges` in `TokenExtensionBadge.stories.tsx` and `TokenExtensionBadges.stories.tsx`. > - **Testing**: > - Add tests for token extension components using Storybook and Vitest in `vitest.workspace.ts` and `TokenExtensionsStatusRow.stories.tsx`. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=solana-foundation%2Fexplorer&utm_source=github&utm_medium=referral)<sup> for d4bec92. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR displays
Token Extensionsfor other accounts.Type of change
Screenshots
Testing
pnpm dev> Visit links belowToken Account: http://localhost:3000/address/7owTEfBdJ2nEdxaUZ2Hm6XevkG7FqHgmeVhkZbUTCCQT/token-extensions
Multisig Account: http://localhost:3000/address/FtrZqgM9begkaLHq3f4hGEUrAK1jjJpkc8hZ1hVk3EDN/token-extensions
Related Issues
Relates to: #516
Checklist
Important
Add feature to display token extensions for accounts, handling different account types and rendering logic in
page-client.tsxandTokenExtensionsCard.tsx.Token Extensionsfor accounts inpage-client.tsxusingTokenExtensionsCard.mintandaccounttypes, renderingTokenExtensionsCardif extensions exist, otherwise showsNoResults.ErrorBoundaryto handle errors inTokenExtensionsEntriesPageClient.TokenExtensionsCardinTokenExtensionsCard.tsxnow fetches token info and displays extensions usingTokenExtensionsSection.populateTokenExtensions()to parse and sort extensions.epochvariable inTokenAccountSection.tsx.TokenExtensionsPageinpage.tsxto useTokenExtensionsEntriesPageClient.This description was created by
for f41dd7e. It will automatically update as commits are pushed.