feat: add clickable badges with extensions to T22 page#556
Merged
ngundotra merged 19 commits intosolana-foundation:masterfrom Apr 17, 2025
Merged
feat: add clickable badges with extensions to T22 page#556ngundotra merged 19 commits intosolana-foundation:masterfrom
ngundotra merged 19 commits intosolana-foundation:masterfrom
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 d4bec92 in 4 minutes and 24 seconds
More details
- Looked at
1334lines of code in21files - Skipped
2files when reviewing. - Skipped posting
11drafted comments based on config settings.
1. app/features/token-extensions/use-token-extension-navigation.ts:72
- Draft comment:
Ensure that adding and removing the 'hashchange' event listener with globalThis is robust across SSR. Consider verifying that 'globalThis' usage is safe in this context. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
2. app/components/account/TokenExtensionsCard.tsx:60
- Draft comment:
The 'populateTokenExtensions' function builds a Map and returns its values; ensure that using Map to dedupe extensions is intended behavior. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
3. vitest.workspace.ts:8
- Draft comment:
Good use of a fallback for __dirname in ESM environments. This pattern is clear and robust. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
4. vitest.workspace.ts:18
- Draft comment:
Nice integration of the Storybook testing plugin using the computed configDir. The use of path.join with dirname is clear. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
5. vitest.workspace.ts:25
- Draft comment:
The nodePolyfills configuration is properly set with globals and 'include'. Consider removing the commented-out config options if not needed to reduce clutter. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%
None
6. vitest.workspace.ts:41
- Draft comment:
The test configuration for running browser tests with Playwright is well defined. Ensure the setup file '.storybook/vitest.setup.ts' is placed at the expected location. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
7. vitest.workspace.ts:51
- Draft comment:
Overall, the workspace configuration is clearly organized and follows Storybook's testing guidelines. No issues found. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
8. app/components/account/TokenExtensionsCard.tsx:37
- Draft comment:
Typo found: The comment on line 37 says 'satisty' but it should be 'satisfy' (or 'satisfies'). Please correct the spelling. - 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/token-extensions/TokenExtensionsStatusRow.tsx:12
- Draft comment:
There's a typographical error on line 12: the word 'compoent' should be corrected to 'component'. - 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 an extremely minor issue in a comment line, not in actual code. Comments are for developer reference and this typo doesn't impact functionality or code quality. This kind of minor correction could be seen as noise in the PR review process.
The typo could indicate a lack of attention to detail, and poor documentation can lead to maintenance issues over time.
While documentation quality is important, this is an extremely minor typo that doesn't affect code understanding, and fixing it provides minimal value compared to the overhead of addressing the comment.
This comment should be deleted as it addresses a trivial issue that doesn't impact code quality or functionality.
10. app/features/token-extensions/use-token-extension-navigation.ts:8
- Draft comment:
Typographical error: In the comment on line 8, 'loose' should be 'lose' to correctly convey that the functionality is not lost. - 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 (loose vs lose), it's an extremely minor issue that doesn't affect code functionality. Comments about typos in other comments are not actionable code changes and don't improve code quality. The rules specifically say to only comment if there is clearly a code change required.
Perhaps poor documentation could lead to maintainability issues in the future? Maybe clarity in comments is important enough to fix?
While clear documentation is important, a minor spelling error that doesn't affect meaning doesn't warrant a PR comment. The meaning is still clear despite the typo.
Delete this comment as it's about a minor spelling error in documentation rather than an actionable code change.
11. package.json:43
- Draft comment:
Typo detected in package name '@solflare-wallet/utl-sdk'. Please verify if it should be '@solflare-wallet/util-sdk'. - 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.
Workflow ID: wflow_6QhraUrkITdqJ5s1
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.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
ngundotra
approved these changes
Apr 17, 2025
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 adds clickable bades with extensions to T22 page.
Type of change
Screenshots
token-extension-navigation.mov
Testing
pnpm tRelated Issues
#534
Checklist
Important
Add clickable badges for token extensions on T22 page with Storybook support and tests.
TokenExtensionsStatusRow.tsxandTokenExtensionsSection.tsx.TokenExtensionBadgeandTokenExtensionBadgescomponents for displaying extension statuses.useTokenExtensionNavigationhook for managing navigation to token extensions..storybook/main.tsand.storybook/preview.tsx.TokenExtensionBadgeandTokenExtensionBadgesinTokenExtensionBadge.stories.tsxandTokenExtensionBadges.stories.tsx.vitest.workspace.tsandTokenExtensionsStatusRow.stories.tsx.This description was created by
for d4bec92. It will automatically update as commits are pushed.