-
Notifications
You must be signed in to change notification settings - Fork 77
fix: name and right button link to deployment page, not the whole line #2093
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: name and right button link to deployment page, not the whole line #2093
Conversation
WalkthroughThe PR refactors deployment row navigation from implicit row-click to explicit button and anchor link elements, adds validation filtering to the selection hook to ensure selected IDs match current data using intersection, and updates checkbox styling for the expanded touch target mode. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/deploy-web/src/components/deployments/DeploymentListRow.tsx (1)
180-182: Consider using semantic link styles instead of hardcoded color.The anchor correctly provides explicit navigation to deployment details, but
className="text-black"hardcodes the text color, which may:
- Not follow the design system's link patterns
- Fail to indicate the element is clickable
- Break in dark mode themes
Consider using a semantic class or removing the className to inherit proper link styling:
- <a href={UrlService.deploymentDetails(deployment.dseq)} className="text-black"> + <a href={UrlService.deploymentDetails(deployment.dseq)} className="hover:underline"> <DeploymentName deployment={deployment} deploymentServices={leaseStatus?.services} providerHostUri={provider?.hostUri} /> </a>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/deploy-web/src/components/deployments/DeploymentListRow.tsx(5 hunks)apps/deploy-web/src/hooks/useListSelection/useListSelection.ts(3 hunks)packages/ui/components/checkbox.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/src/components/deployments/DeploymentListRow.tsxpackages/ui/components/checkbox.tsxapps/deploy-web/src/hooks/useListSelection/useListSelection.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/src/components/deployments/DeploymentListRow.tsxpackages/ui/components/checkbox.tsxapps/deploy-web/src/hooks/useListSelection/useListSelection.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1994
File: apps/deploy-web/src/hooks/useListSelection/useListSelection.spec.ts:58-64
Timestamp: 2025-10-17T10:04:00.909Z
Learning: In the Akash Console deployment list multi-select feature (PR #1994, apps/deploy-web/src/hooks/useListSelection), the requirement is for additive Shift+click behavior: subsequent Shift+clicks should expand/add to the existing selection rather than replacing the previous Shift range. This differs from canonical Gmail/Finder-style replacement behavior.
🧬 Code graph analysis (2)
apps/deploy-web/src/components/deployments/DeploymentListRow.tsx (1)
apps/deploy-web/src/components/deployments/DeploymentName/DeploymentName.tsx (1)
DeploymentName(27-69)
packages/ui/components/checkbox.tsx (1)
apps/provider-console/src/utils/styleUtils.ts (1)
cn(4-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (7)
packages/ui/components/checkbox.tsx (1)
33-33: LGTM! Styling update aligns with navigation refactor.The removal of negative margin and switch to
hover:bg-accentimproves consistency with the updated navigation pattern in the deployment list.apps/deploy-web/src/components/deployments/DeploymentListRow.tsx (3)
91-93: LGTM! Navigation callback is correctly implemented.The
viewDeploymentcallback is properly memoized and uses the appropriate routing service.
167-167: LGTM! Row-level navigation correctly removed.The TableRow now provides visual hover feedback without implicit navigation, aligning with the PR objective.
344-346: LGTM! Navigation button correctly implements explicit navigation.The Button properly wraps the NavArrowRight icon and calls the
viewDeploymenthandler, providing clear, explicit navigation as intended by the PR.apps/deploy-web/src/hooks/useListSelection/useListSelection.ts (3)
2-3: LGTM! Per-method imports optimize bundle size.The imports use the recommended per-method pattern for lodash, ensuring only the required utilities are included in the bundle.
41-50: LGTM! Toggle selection logic is correct.The callback properly handles single-item selection toggling and maintains the interval selection anchor for shift-click behavior.
83-85: Excellent defensive improvement! Selection validation prevents stale selections.The
validSelectedItemIdscomputation filters out any selected IDs that no longer exist in the currentidsarray. This prevents edge cases where:
- Items are removed from the list but remain in the selection state
- The list is filtered or refreshed with a different set of items
The memoization correctly depends on both
idsandselectedItemIds, ensuring the intersection is recomputed when either changes.Also applies to: 89-89, 94-94
refs #1981
Summary by CodeRabbit
Bug Fixes
Style