Resolve bounty dashboard and accessibility issues - #230
Closed
Ishant5436 wants to merge 2 commits into
Closed
Ishant5436 wants to merge 2 commits into
Ishant5436 wants to merge 2 commits into
Conversation
- Extracted MobileCTA into its own file - Extracted shared state into useBountyCTAState hook - Updated bounty-detail-client.tsx to import MobileCTA from new file
Resolves Issue 216: Add missing aria-labels to icon-only buttons for accessibility compliance. Resolves Issue 212: Standardize loading skeleton implementation and consolidate into components/ui/loading. Resolves Issue 205: Implement discrete maintainer actions and UI loading states in model4-maintainer-dashboard.
|
@Ishant5436 is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR refactors loading UI and bounty detail call-to-action logic: skeleton components are consolidated into ChangesSkeleton UI Centralization and Bounty CTA Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/bounty-detail/model4-maintainer-dashboard.tsx (1)
41-46: ⚡ Quick winGuard
loadingActionreset withfinallyin async handlers.If these handlers later call real APIs and one throws,
loadingActioncan remain stuck and keep controls disabled.Suggested pattern
const handleMessage = async (userName: string) => { - setLoadingAction(`Message-${userName}`); - await new Promise((r) => setTimeout(r, 800)); - toast.success(`Message sent to ${userName}`); - setLoadingAction(null); + setLoadingAction(`Message-${userName}`); + try { + await new Promise((r) => setTimeout(r, 800)); + toast.success(`Message sent to ${userName}`); + } finally { + setLoadingAction(null); + } };Also applies to: 48-53, 55-60, 62-67, 69-74
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/bounty-detail/model4-maintainer-dashboard.tsx` around lines 41 - 46, The async handlers that set loadingAction (e.g., handleMessage) must ensure loadingAction is always cleared even if an awaited call throws; wrap the async work in try/finally and move setLoadingAction(null) into the finally block so the spinner/disabled state cannot get stuck, keeping the toast and awaits inside the try and preserving the initial setLoadingAction(...) before the try; apply the same change to all other async handlers that set loadingAction in this file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/bounty-detail/bounty-detail-mobile-cta.tsx`:
- Around line 176-178: The mobile cancel dialog currently preserves previous
input because closing via AlertDialogCancel doesn't clear the cancelReason
state; update the component so dismissing the dialog resets the reason—either
add an onClick handler to AlertDialogCancel that calls setCancelReason('') (and
keep disabled tied to isCancelling) or handle the dialog's open state change
(e.g., onOpenChange) to detect when it closes and call setCancelReason('');
reference AlertDialogCancel, isCancelling, cancelReason, and setCancelReason
when making the change.
In `@components/bounty-detail/model4-maintainer-dashboard.tsx`:
- Around line 42-43: The loadingAction key currently uses userName which can
collide for contributors with identical display names; update all calls that
setLoadingAction (e.g., setLoadingAction(`Message-${userName}`) and similar
occurrences) to use the unique contributor.userId (e.g.,
`Message-${contributor.userId}`) while leaving userName only for toast/display
text; apply the same change to every other instance mentioned (lines
corresponding to the other setLoadingAction calls) so all loading keys are built
from contributor.userId to avoid cross-row collisions.
---
Nitpick comments:
In `@components/bounty-detail/model4-maintainer-dashboard.tsx`:
- Around line 41-46: The async handlers that set loadingAction (e.g.,
handleMessage) must ensure loadingAction is always cleared even if an awaited
call throws; wrap the async work in try/finally and move setLoadingAction(null)
into the finally block so the spinner/disabled state cannot get stuck, keeping
the toast and awaits inside the try and preserving the initial
setLoadingAction(...) before the try; apply the same change to all other async
handlers that set loadingAction in this file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11a3f33b-acb8-451f-a8c0-2b9f47bd2b45
📒 Files selected for processing (16)
app/bounty/page.tsxapp/wallet/page.tsxcomponents/bounty-detail/bounty-detail-client.tsxcomponents/bounty-detail/bounty-detail-mobile-cta.tsxcomponents/bounty-detail/bounty-detail-sidebar-cta.tsxcomponents/bounty-detail/model4-maintainer-dashboard.tsxcomponents/bounty-detail/use-bounty-cta-state.tscomponents/bounty/bounty-card-skeleton.tsxcomponents/bounty/bounty-grid.tsxcomponents/bounty/bounty-list.tsxcomponents/leaderboard/leaderboard-table.tsxcomponents/mode-toggle.tsxcomponents/ui/loading/bounty-card-skeleton.tsxcomponents/ui/loading/bounty-detail-skeleton.tsxcomponents/ui/loading/leaderboard-row-skeleton.tsxcomponents/ui/loading/wallet-page-skeleton.tsx
💤 Files with no reviewable changes (1)
- components/bounty/bounty-card-skeleton.tsx
Comment on lines
+176
to
+178
| <AlertDialogCancel disabled={isCancelling}> | ||
| Keep Bounty | ||
| </AlertDialogCancel> |
There was a problem hiding this comment.
Reset cancel reason when dismissing the mobile cancel dialog.
Closing with “Keep Bounty” currently preserves the previous reason text, so reopening can submit stale input unintentionally.
Suggested fix
- <AlertDialogCancel disabled={isCancelling}>
+ <AlertDialogCancel
+ disabled={isCancelling}
+ onClick={() => setCancelReason("")}
+ >
Keep Bounty
</AlertDialogCancel>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/bounty-detail/bounty-detail-mobile-cta.tsx` around lines 176 -
178, The mobile cancel dialog currently preserves previous input because closing
via AlertDialogCancel doesn't clear the cancelReason state; update the component
so dismissing the dialog resets the reason—either add an onClick handler to
AlertDialogCancel that calls setCancelReason('') (and keep disabled tied to
isCancelling) or handle the dialog's open state change (e.g., onOpenChange) to
detect when it closes and call setCancelReason(''); reference AlertDialogCancel,
isCancelling, cancelReason, and setCancelReason when making the change.
Comment on lines
+42
to
+43
| setLoadingAction(`Message-${userName}`); | ||
| await new Promise((r) => setTimeout(r, 800)); |
There was a problem hiding this comment.
Use userId (not userName) in loadingAction keys to avoid collisions.
Two contributors can share the same display name, which can bind loading indicators to the wrong row/action. Build the key from contributor.userId and keep userName only for toast text.
Suggested fix
- const handleReleasePayment = async (userName: string) => {
- setLoadingAction(`Release Payment-${userName}`);
+ const handleReleasePayment = async (userId: string, userName: string) => {
+ setLoadingAction(`release-payment:${userId}`);
await new Promise((r) => setTimeout(r, 1200));
toast.success(`Payment released for ${userName}`);
setLoadingAction(null);
};- onClick={() => handleReleasePayment(contributor.userName)}
+ onClick={() => handleReleasePayment(contributor.userId, contributor.userName)}
- {loadingAction === `Release Payment-${contributor.userName}` ? (
+ {loadingAction === `release-payment:${contributor.userId}` ? (Also applies to: 49-50, 56-57, 63-64, 70-71, 205-206, 226-227
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/bounty-detail/model4-maintainer-dashboard.tsx` around lines 42 -
43, The loadingAction key currently uses userName which can collide for
contributors with identical display names; update all calls that
setLoadingAction (e.g., setLoadingAction(`Message-${userName}`) and similar
occurrences) to use the unique contributor.userId (e.g.,
`Message-${contributor.userId}`) while leaving userName only for toast/display
text; apply the same change to every other instance mentioned (lines
corresponding to the other setLoadingAction calls) so all loading keys are built
from contributor.userId to avoid cross-row collisions.
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
2 participants
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.
Resolves Issue 216: Add missing aria-labels to icon-only buttons for accessibility compliance.\nResolves Issue 212: Standardize loading skeleton implementation and consolidate into components/ui/loading.\nResolves Issue 205: Implement discrete maintainer actions and UI loading states in model4-maintainer-dashboard.
Summary by CodeRabbit
New Features
Refactor
Style