feat: token 2022 page rework#516
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. |
| ## Development | ||
|
|
||
| ### Creating new UI components | ||
|
|
||
| For new components we use [shadcn/ui](https://ui.shadcn.com/docs). | ||
|
|
||
| To generate a component, use this script: | ||
|
|
||
| ```bash | ||
| pnpm gen accordion | ||
| ``` | ||
|
|
||
| It translates needed component into `pnpx shadcn@version add accordion` and installs it. No newline at end of file |
There was a problem hiding this comment.
nit: this is a bit confusing, what was this used to generate?
Do developers need to use this, or is this just recommended flow for creating new components on the Explorer?
There was a problem hiding this comment.
Second one. I've created an accordion, a tooltip, and a new badge with shadcn/ui and this is the recommended way to generate such components. Didn't want to leave just a script with shadcn without some words about it
|
Still missing a toggeable background for |
fixed |
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to a8aa778 in 3 minutes and 47 seconds
More details
- Looked at
2279lines of code in28files - Skipped
24files when reviewing. - Skipped posting
15drafted comments based on config settings.
1. app/components/common/TableCardBody.tsx:16
- Draft comment:
Duplicate declaration for TableCardBodyProps – consider merging the two interface definitions to avoid conflicts. - 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.
2. app/components/account/TokenAccountSection.tsx:642
- Draft comment:
TokenExtensionRow is defined as a function (not a React component) and used directly for rendering. Consider refactoring it into a proper React component for better readability and lifecycle management. - Reason this comment was not posted:
Comment was on unchanged code.
3. app/components/account/__test__/TokenExtensionRow.spec.tsx:14
- Draft comment:
Mocking 'next/navigation' and calling TokenExtensionRow directly may obscure potential issues when integrated as a React component. Ensure tests remain robust if refactored into a standard component. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =20%<= threshold50%
The comment suggests ensuring tests remain robust if the code is refactored, which violates the rule against asking the PR author to ensure testing. It doesn't provide a specific suggestion or point out a specific issue with the code.
4. app/prettierrc.cjs:5
- Draft comment:
Ensure that prettierConfigSolana.plugins is always an array – using the nullish operator with concat might hide cases where plugins is undefined. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
5. app/components/common/TableCardBody.tsx:16
- Draft comment:
Duplicate interface declaration for TableCardBodyProps detected. Merge the two definitions (one with optional headerComponent) into a single interface. - 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. .prettierrc.cjs:5
- Draft comment:
Optional: Consider using the spread operator instead of concatenating an array. For example, useplugins: [...(prettierConfigSolana.plugins || []), 'prettier-plugin-tailwindcss']. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
This is a style preference in a configuration file. Both approaches work equally well functionally. The spread operator version is more modern and arguably more readable, but the current code is not wrong or problematic. This feels like a minor stylistic preference rather than a significant improvement.
The current code is perfectly functional and clear. The suggestion might actually be worse since it changes from nullish coalescing (??) to logical OR (||), which behaves differently with falsy values.
While the spread operator might be more modern, making this change would be purely cosmetic and doesn't meaningfully improve code quality or maintainability.
Delete this comment as it suggests a purely stylistic change that doesn't materially improve the code.
7. app/components/account/__test__/TokenExtensionRow.spec.tsx:32
- Draft comment:
Consider invoking the TokenExtensionRow as a component (<TokenExtensionRow ... />) rather than calling it as a plain function. This is more conventional for React components. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
8. app/components/account/TokenAccountSection.tsx:1012
- Draft comment:
Typo: The key prop in the additional metadata map is written as key="{keyValuePair[0]}". It should be key={keyValuePair[0]} so that the value is used properly. - 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/TokenExtensionsSection.tsx:150
- Draft comment:
Typographical error: In the comment on line 150, 'tootip's position' should be corrected to 'tooltip's position'. - 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, it's just pointing out a spelling error in a comment, not in actual code. Comments are for developer documentation and this typo doesn't impact functionality. The rules state we should only keep comments that require code changes, and this is more of a trivial documentation fix.
The typo could make the comment slightly less clear for future developers. Also, maintaining good documentation quality could be considered important.
However, this is an extremely minor issue that doesn't affect code functionality or significantly impact readability. The meaning is still clear despite the typo.
Delete this comment as it's too minor and doesn't require any actual code changes - it only affects a documentation comment.
10. app/components/shared/ui/badge.tsx:57
- Draft comment:
Typographical error: Duplicate 'dark:' modifier observed in the 'destructive' variant class string. Please remove the extra 'dark:' prefixes to ensure consistency. - Reason this comment was not posted:
Marked as duplicate.
11. app/scss/dashkit/_navbar.scss:559
- Draft comment:
Typographical error: The comment 'Creates a small vertion of the vertical navbar' contains a typo. Please change 'vertion' to 'version'. - 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.
12. app/scss/dashkit/_variables.scss:482
- Draft comment:
Typographical issue: The variable declaration for$pagination-hover-border-colorincludes!defaulttwice. Please remove the duplicate 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.
13. app/scss/dashkit/_variables.scss:159
- Draft comment:
Typographical issue: In the comment for$spacer * 9(expected to equal 216px), there are extra commas (216px,,,,). It might be clearer to remove the extra commas unless they were intentional, to avoid confusion. - 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.
14. docs/development.md:13
- Draft comment:
In line 13, the phrase "It translates needed component into ..." could be improved for clarity. Consider changing it to "It translates the necessary component into ..." or "It translates the component name into ..." to make the intent clearer. - 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%
Documentation clarity is important, but this suggestion is extremely minor and subjective. The existing wording "needed component" is understandable and grammatically correct. The suggested alternatives don't significantly improve clarity. PR comments should focus on substantial issues rather than minor wording preferences.
Documentation clarity directly impacts developer experience, so maybe any improvement in clarity is worth suggesting?
While clarity is important, this change is too minor to warrant a PR comment. The existing wording is already clear enough for developers to understand.
Delete this comment as it suggests an extremely minor wording change that doesn't significantly improve clarity or fix any real issues.
15. postcss.config.mjs:7
- Draft comment:
It appears that the file is missing a trailing newline at the end. Could you please add an empty newline at the end of the file? - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 40% vs. threshold = 50%
Missing trailing newlines can cause issues with some tools and is generally considered good practice. Since this is a new file being added, pointing this out now is better than letting it persist. The issue is clear and actionable. However, many build tools and linters would catch this automatically.
This might be too minor of an issue to warrant a PR comment. Many editors automatically add trailing newlines, and build tools often enforce this.
While minor, it's a clear, objective issue with a simple fix that improves code quality. The fact that it's a new file makes it worth addressing now.
Keep the comment as it points out a real, actionable issue in a newly added file, though with a relatively low confidence score due to its minor nature.
Workflow ID: wflow_9Ez3421nSeg8gxTB
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.
| ); | ||
| } | ||
|
|
||
| function TokenExtensionsLink({ address, tab }: { address: string; tab: Tab }) { |
There was a problem hiding this comment.
This component duplicates the existing Tab component. Consider using Tab instead, passing tab.path and tab.title as separate parameters.
- function
Tab(layout.tsx)
There was a problem hiding this comment.
That's wrong. Tab has different logic for isActive inside it. Accepting this will lead to incorrect state for tabs
There was a problem hiding this comment.
might be a good idea to rename Tab to something more specific or to bypass isActive through props
## Description This PR displays `Token Extensions` for other accounts. ## Type of change - [x] New feature ## Screenshots <img width="732" alt="image" src="https://github.com/user-attachments/assets/44de309c-f2cf-43f9-9f29-19391be764e5" /> <img width="739" alt="image" src="https://github.com/user-attachments/assets/d24eacd5-2715-4683-9667-6c2c8808f2cc" /> ## Testing `pnpm dev` > Visit links below Token Account: http://localhost:3000/address/7owTEfBdJ2nEdxaUZ2Hm6XevkG7FqHgmeVhkZbUTCCQT/token-extensions Multisig Account: http://localhost:3000/address/FtrZqgM9begkaLHq3f4hGEUrAK1jjJpkc8hZ1hVk3EDN/token-extensions ## Related Issues Relates to: #516 ## 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] CI/CD checks pass - [x] I have included screenshots for protocol screens (if applicable) <!-- ELLIPSIS_HIDDEN --> ---- > [!IMPORTANT] > Add feature to display token extensions for accounts, handling different account types and rendering logic in `page-client.tsx` and `TokenExtensionsCard.tsx`. > > - **Behavior**: > - Displays `Token Extensions` for accounts in `page-client.tsx` using `TokenExtensionsCard`. > - Handles `mint` and `account` types, rendering `TokenExtensionsCard` if extensions exist, otherwise shows `NoResults`. > - Uses `ErrorBoundary` to handle errors in `TokenExtensionsEntriesPageClient`. > - **Components**: > - `TokenExtensionsCard` in `TokenExtensionsCard.tsx` now fetches token info and displays extensions using `TokenExtensionsSection`. > - Introduces `populateTokenExtensions()` to parse and sort extensions. > - **Misc**: > - Removes unused `epoch` variable in `TokenAccountSection.tsx`. > - Updates `TokenExtensionsPage` in `page.tsx` to use `TokenExtensionsEntriesPageClient`. > > <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 f41dd7e. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
Description
This PR improves "Token Extensions" token page. Goal: better display for active extensions.
Type of change
Screenshots
Testing
pnpm tpnpm formatRelated Issues
None.
Checklist
Important
Enhances the Token 2022 page with a new UI for displaying active token extensions, including new components, styles, and configuration updates.
TokenExtensionsPageinpage.tsxto display token extensions for a given address.TokenExtensionsCardandTokenExtensionsSectioncomponents to render token extension details.TokenExtensionRowinTokenAccountSection.tsxto display individual token extension data.TokenExtensionRowinTokenExtensionRow.spec.tsx.Accordion,Badge, andTooltipcomponents inui/accordion.tsx,ui/badge.tsx, andui/tooltip.tsx.StatusBadgeinStatusBadge.tsxto indicate the status of token extensions.tailwind.config.tsandstyles.css.dashkitto remove unused styles and adjust navbar styles..prettierrc.cjsfor Prettier configuration with Tailwind CSS plugin..eslintrc.jsonto disable@typescript-eslint/no-non-null-assertionrule.postcss.config.mjsfor PostCSS configuration.@radix-ui/react-accordion,@radix-ui/react-tooltip, andtailwind-mergetopackage.json.package.jsonscripts to includepnpm genfor generating UI components.This description was created by
for a8aa778. It will automatically update as commits are pushed.