-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Improve reward UI/UX #2504
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
base: main
Are you sure you want to change the base?
Improve reward UI/UX #2504
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update introduces a comprehensive refactor of the reward system, transitioning from a model with a single default reward per program to one supporting multiple default rewards categorized by event types ("click", "lead", "sale"). The changes affect database schemas, backend logic, API endpoints, frontend components, and validation schemas to consistently support event-based default rewards and partner-specific reward associations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant API
participant DB
User->>Frontend: Request program details
Frontend->>API: GET /programs/[programId]
API->>DB: Query Program (include default rewards by event)
DB-->>API: Return Program with rewards [{event, type, amount, default}]
API-->>Frontend: Return Program + rewards[]
Frontend->>User: Display default rewards for click, lead, sale
User->>Frontend: Create new reward (for event type)
Frontend->>API: POST /rewards (with isDefault, event)
API->>DB: Validate uniqueness, create reward, update enrollments
DB-->>API: Confirmation
API-->>Frontend: Reward created
User->>Frontend: Enroll partner with reward
Frontend->>API: POST /enroll-partner (with reward for event)
API->>DB: Upsert ProgramEnrollment, set event-specific rewardId
DB-->>API: Enrollment confirmation
API-->>Frontend: Enrollment success
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
… retrieval logic in partner profile components.
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
- Introduced `sortRewardsByEvent` function to organize rewards by event type. - Updated `getProgramViaEdge` to fetch rewards more efficiently and return them in a structured format. - Modified the API response in `route.tsx` to utilize the new rewards sorting logic. - Adjusted the rendering logic to display the first reward based on the sorted list.
- Renamed parameters for clarity, changing `includeDefaultReward` and `includeRewards` to `includeDefaultRewards`. - Updated the logic in `getProgramOrThrow` to utilize the new `sortRewardsByEvent` function for sorting rewards. - Adjusted API response handling in `route.ts` to reflect the updated reward inclusion logic.
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please 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: 2
🧹 Nitpick comments (1)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/rewards/rewards.tsx (1)
28-38
: Consider iterating overREWARD_EVENTS
instead of three manualfind
callsHard-coding separate look-ups for
"click"
,"lead"
,"sale"
introduces duplication and will break if new event types are added.const defaultRewardsByEvent = Object.keys(REWARD_EVENTS).reduce< Record<EventType, RewardProps | undefined> >((acc, event) => { acc[event as EventType] = rewards?.find( (r) => r.event === event && r.default, ); return acc; }, {} as any);Then render via
Object.entries(defaultRewardsByEvent)
to keep the mapping single-source.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(ee)/partners.dub.co/(dashboard)/programs/[programSlug]/apply/page.tsx
(1 hunks)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/discounts/page-client.tsx
(3 hunks)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/rewards/rewards.tsx
(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/discounts/page-client.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(ee)/partners.dub.co/(dashboard)/programs/[programSlug]/apply/page.tsx
🔇 Additional comments (1)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/rewards/rewards.tsx (1)
233-241
: Verify the generic parameter forusePartnersCount
usePartnersCount
already returns a typed object; providing an explicit<T>
for “array of reward with partnersCount” may not match the hook’s signature and could widen types inadvertently. Double-check the hook definition—if it infers correctly, drop the generic for cleaner code:const { partnersCount, loading: partnersCountLoading } = usePartnersCount({ groupBy: REWARD_EVENT_COLUMN_MAPPING[event], enabled: !isDefault, });
const partnerCount = | ||
!isDefault && reward | ||
? (partnersCount || []).find((r) => r.id === reward.id)?.partnersCount | ||
: undefined; |
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.
Guard against undefined
partner counts before rendering
partnerCount
can be undefined
when the hook returns an array that doesn’t yet include the current reward. The template string then literally prints “undefined partners”.
- const partnerCount =
- !isDefault && reward
- ? (partnersCount || []).find((r) => r.id === reward.id)?.partnersCount
- : undefined;
+ const partnerCount =
+ !isDefault && reward
+ ? (partnersCount || []).find((r) => r.id === reward.id)?.partnersCount ?? 0
+ : 0;
...
- <Badge variant="green">
- {partnerCount} {pluralize("partner", partnerCount || 0)}
- </Badge>
+ <Badge variant="green">
+ {partnerCount} {pluralize("partner", partnerCount)}
+ </Badge>
This guarantees a numeric value and prevents the UI from showing “undefined partners”.
Also applies to: 293-295
🤖 Prompt for AI Agents
In apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/rewards/rewards.tsx
around lines 243 to 246, the variable partnerCount can be undefined, causing the
UI to display "undefined partners". To fix this, ensure partnerCount is always a
number by providing a default value (e.g., 0) when the find operation returns
undefined. Apply the same fix to lines 293 to 295 to prevent similar issues.
<div | ||
className="flex cursor-pointer items-center gap-4 rounded-lg border border-neutral-200 p-4 transition-all hover:border-neutral-300" | ||
onClick={() => setIsOpen(true)} | ||
> | ||
<div className="flex size-10 items-center justify-center rounded-full border border-neutral-200 bg-white"> | ||
<Icon className="size-4 text-neutral-600" /> | ||
</div> | ||
<div className="flex flex-1 items-center justify-between"> |
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.
🛠️ Refactor suggestion
Add basic accessibility props to the clickable container
The outer <div>
acts like a button but lacks role="button"
and keyboard focus/activation (tabIndex={0}
, onKeyDown
). Screen-reader and keyboard users won’t be able to activate the sheet.
- <div
- className="flex cursor-pointer items-center gap-4 ..."
- => setIsOpen(true)}
- >
+ <div
+ role="button"
+ tabIndex={0}
+ => (e.key === "Enter" || e.key === " ") && setIsOpen(true)}
+ className="flex cursor-pointer items-center gap-4 ..."
+ => setIsOpen(true)}
+ >
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<div | |
className="flex cursor-pointer items-center gap-4 rounded-lg border border-neutral-200 p-4 transition-all hover:border-neutral-300" | |
onClick={() => setIsOpen(true)} | |
> | |
<div className="flex size-10 items-center justify-center rounded-full border border-neutral-200 bg-white"> | |
<Icon className="size-4 text-neutral-600" /> | |
</div> | |
<div className="flex flex-1 items-center justify-between"> | |
<div | |
role="button" | |
tabIndex={0} | |
onKeyDown={(e) => | |
(e.key === "Enter" || e.key === " ") && setIsOpen(true) | |
} | |
className="flex cursor-pointer items-center gap-4 rounded-lg border border-neutral-200 p-4 transition-all hover:border-neutral-300" | |
onClick={() => setIsOpen(true)} | |
> | |
<div className="flex size-10 items-center justify-center rounded-full border border-neutral-200 bg-white"> | |
<Icon className="size-4 text-neutral-600" /> | |
</div> | |
<div className="flex flex-1 items-center justify-between"> |
🤖 Prompt for AI Agents
In apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/rewards/rewards.tsx
around lines 252 to 259, the outer div acting as a clickable container lacks
accessibility attributes. Add role="button" to indicate its button behavior, set
tabIndex={0} to make it focusable via keyboard, and implement an onKeyDown
handler to trigger setIsOpen(true) when Enter or Space keys are pressed,
enabling keyboard activation and improving accessibility.
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
🤖 Bug0 QA Agent Here are the results of the automated tests for PR #2504:
To re-run the tests, please comment |
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores