✨ Add brandable event card and update UI for closed and scheduled poll status#2315
Merged
✨ Add brandable event card and update UI for closed and scheduled poll status#2315
Conversation
♻️ Only apply poll branding when primary color is set
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRemoved ScheduledEvent usage from pages, adjusted English poll translation keys, refactored event metadata and poll UI markup, introduced client-side ResponsiveResults with scheduled/closed empty states and Add-to-Calendar, and tweaked theme & shared UI defaults. (50 words) Changes
Sequence DiagramsequenceDiagram
participant Client
participant ResponsiveResults
participant PollCtx as usePoll()
participant DayJS as useDayjs()/dayjs
participant AddCal as AddToCalendarButton
participant PollView as DesktopPoll/MobilePoll
Client->>ResponsiveResults: mount
ResponsiveResults->>PollCtx: read poll state
alt poll.status == "scheduled" && poll.event && !dismissed
ResponsiveResults->>DayJS: format start/end (timezone)
ResponsiveResults->>Client: render scheduled empty-state + AddToCalendar + "View results"
Client->>AddCal: click AddToCalendar
Client->>ResponsiveResults: click "View results"
ResponsiveResults->>ResponsiveResults: set dismissed = true
else poll.status == "closed" && !dismissed
ResponsiveResults->>Client: render closed empty-state + "View results"
Client->>ResponsiveResults: click "View results"
ResponsiveResults->>ResponsiveResults: set dismissed = true
else
ResponsiveResults->>PollView: render poll UI (Desktop or Mobile)
end
PollView-->>Client: interactive poll/results
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
lukevella
force-pushed
the
ral-1125-event-card-space-branding
branch
from
aa0d617 to
2157ed4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/src/components/event-card.tsx (1)
17-38: Semantic HTML issue:<div>wrapper inside<dl>breaks definition list structure.The
<div>wrapper around<dt>and<dd>inIconDescriptiontechnically violates the HTML spec where<dl>should only contain<dt>,<dd>,<div>, or<script>/<template>elements as direct children. While modern browsers are tolerant, the current structure could cause accessibility issues with screen readers.However, HTML5 does allow
<div>as a grouping element inside<dl>. The current implementation is valid but could be cleaner.💡 Optional: Simplify by removing the extra wrapper
function IconDescription({ icon, label, }: { icon: React.ReactNode; label: React.ReactNode; }) { return ( - <div className="flex items-center gap-1"> - <dt>{icon}</dt> - <dd>{label}</dd> - </div> + <> + <dt className="inline-flex items-center">{icon}</dt> + <dd className="ml-1">{label}</dd> + </> ); }Alternatively, keep the
<div>since HTML5 permits it inside<dl>for styling purposes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/event-card.tsx` around lines 17 - 38, The IconDescription currently wraps <dt> and <dd> in a <div>, which can break the semantic structure under the <dl> rendered by IconDescriptionList; update the IconDescription component to remove the extra wrapper so it returns the <dt> and <dd> as siblings (or a React fragment) and move any styling from the removed <div> onto the <dt> and <dd> (or use a wrapping element allowed inside <dl> if grouping is required), ensuring IconDescriptionList still receives valid direct children.apps/web/src/components/event-meta.tsx (1)
28-31: Inconsistentcn()argument ordering -classNamepositioned first prevents overrides.In
EventMetaDescription,classNameis placed before the base classes incn(). With Tailwind's cascade, the last-listed conflicting class wins, so passing a customclassNamewon't be able to override the base styles likemt-2ortext-sm.Compare with
EventMetaTitle(line 13) whereclassNameis appended last, allowing overrides.💡 Suggested fix for consistent override behavior
export function EventMetaDescription({ className, children, }: { className?: string; children: React.ReactNode; }) { return ( <p className={cn( - className, "mt-2 min-w-0 whitespace-pre-wrap text-pretty text-muted-foreground text-sm leading-relaxed", + className, )} > <TruncatedLinkify>{children}</TruncatedLinkify> </p> ); }Apply the same pattern to
EventMetaList,EventMetaItem,EventMetaHost, andEventMetaHostNameif custom class overrides should be supported.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/event-meta.tsx` around lines 28 - 31, EventMetaDescription places the incoming className before the base Tailwind classes in the cn(...) call which prevents callers from overriding base styles; update the cn(...) invocation in EventMetaDescription to pass className as the last argument (after the base classes) so custom classes win, and apply the same change to EventMetaList, EventMetaItem, EventMetaHost, and EventMetaHostName where override behavior should be supported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/poll/responsive-results.tsx`:
- Around line 70-96: Update the GetPollApiResponse type to include the missing
nullable event property that mirrors what polls.get returns: add event: { id:
string; start: Date; duration: number; attendees: Array<{ name: string; email:
string; status: string }>; status: string } | null so components like
ResponsiveResults (responsive-results.tsx) using poll.event and
ScheduledDateTime/AddToCalendarButton no longer violate TypeScript; ensure the
exported type used by the trpc client/types.ts (GetPollApiResponse) matches the
router's constructed scheduledEvent shape and update any related imports or test
types accordingly.
---
Nitpick comments:
In `@apps/web/src/components/event-card.tsx`:
- Around line 17-38: The IconDescription currently wraps <dt> and <dd> in a
<div>, which can break the semantic structure under the <dl> rendered by
IconDescriptionList; update the IconDescription component to remove the extra
wrapper so it returns the <dt> and <dd> as siblings (or a React fragment) and
move any styling from the removed <div> onto the <dt> and <dd> (or use a
wrapping element allowed inside <dl> if grouping is required), ensuring
IconDescriptionList still receives valid direct children.
In `@apps/web/src/components/event-meta.tsx`:
- Around line 28-31: EventMetaDescription places the incoming className before
the base Tailwind classes in the cn(...) call which prevents callers from
overriding base styles; update the cn(...) invocation in EventMetaDescription to
pass className as the last argument (after the base classes) so custom classes
win, and apply the same change to EventMetaList, EventMetaItem, EventMetaHost,
and EventMetaHostName where override behavior should be supported.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 38b73548-659e-47a8-a8f8-8bfea837df84
📒 Files selected for processing (12)
apps/web/public/locales/en/app.jsonapps/web/src/app/[locale]/(optional-space)/poll/[urlId]/admin-page.tsxapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsxapps/web/src/components/event-card.tsxapps/web/src/components/event-meta.tsxapps/web/src/components/poll/desktop-poll.tsxapps/web/src/components/poll/desktop-poll/participant-row.tsxapps/web/src/components/poll/mobile-poll.tsxapps/web/src/components/poll/responsive-results.tsxpackages/tailwind-config/shared-styles.csspackages/ui/src/avatar.tsxpackages/ui/src/card.tsx
💤 Files with no reviewable changes (3)
- apps/web/src/app/[locale]/invite/[urlId]/invite-page.tsx
- apps/web/src/app/[locale]/(optional-space)/poll/[urlId]/admin-page.tsx
- apps/web/src/components/poll/mobile-poll.tsx
lukevella
force-pushed
the
ral-1125-event-card-space-branding
branch
from
b5ff57a to
1175195
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/event-card.tsx`:
- Around line 84-101: The response-options column uses a fixed "w-1/3" width
causing it to stay one-third on phones; update the wrapper div (currently
className="w-1/3") to be responsive, e.g. className="w-full md:w-1/3", so it is
full width below md; also check the IconDescriptionList/IconDescription usage
for any "whitespace-nowrap" classes and remove or replace them with
wrapping-friendly utilities so the list doesn't overflow on small screens.
- Around line 17-23: IconDescriptionList currently only types and accepts
children so any DOM props like aria-label are dropped; change its props to
accept native <dl> attributes (e.g., use React.HTMLAttributes<HTMLDListElement>
or similar) and spread the remaining props onto the <dl> element so aria-label
and other attributes are forwarded (update the function signature and use
{...props} on the <dl>). Also add the missing defaults props to the three Trans
components referenced (the ones around lines 91/94/97) to match the pattern on
line 86: use <Trans i18nKey="yes" defaults="Yes" />, <Trans i18nKey="ifNeedBe"
defaults="If need be" />, and <Trans i18nKey="no" defaults="No" />.
In `@apps/web/src/components/poll/responsive-results.tsx`:
- Around line 39-45: The client-side <Trans> usage in the duration === 0 branch
(responsive-results.tsx) lacks a defaults prop, which can cause an empty line
when the locale is missing the "allDay" key; update the <Trans> invocation
inside the duration === 0 return to include a sensible fallback via the defaults
prop (e.g., defaults="All day") so the scheduled state always shows a value,
keeping the change local to the block that renders when duration === 0.
In `@packages/ui/src/avatar.tsx`:
- Around line 7-19: Restore the shared Avatar default border by adding back
"ring-1 ring-button-outline" into the base classes of avatarVariants so the
primitive keeps contrast (update the cva call used to build avatarVariants). For
the one-off branded event card that needs a flush avatar, explicitly opt out
locally in that component (the branded card that composes Avatar) by overriding
classes with a prefixed utility such as "ring-0" or "ring-transparent" when
rendering Avatar; check the consumers that rely on the default (space-icon and
optimized-avatar-image usages) to ensure they keep the default and only the
branded card overrides the ring.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a05f3572-8913-4805-8a00-f6f8685c6cc8
📒 Files selected for processing (12)
apps/web/public/locales/en/app.jsonapps/web/src/app/[locale]/(optional-space)/poll/[urlId]/admin-page.tsxapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsxapps/web/src/components/event-card.tsxapps/web/src/components/event-meta.tsxapps/web/src/components/poll/desktop-poll.tsxapps/web/src/components/poll/desktop-poll/participant-row.tsxapps/web/src/components/poll/mobile-poll.tsxapps/web/src/components/poll/responsive-results.tsxpackages/tailwind-config/shared-styles.csspackages/ui/src/avatar.tsxpackages/ui/src/card.tsx
💤 Files with no reviewable changes (2)
- apps/web/src/app/[locale]/invite/[urlId]/invite-page.tsx
- apps/web/src/app/[locale]/(optional-space)/poll/[urlId]/admin-page.tsx
✅ Files skipped from review due to trivial changes (3)
- apps/web/src/components/poll/desktop-poll/participant-row.tsx
- apps/web/src/components/poll/desktop-poll.tsx
- apps/web/src/components/poll/mobile-poll.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/ui/src/card.tsx
- packages/tailwind-config/shared-styles.css
- apps/web/public/locales/en/app.json
- apps/web/src/components/event-meta.tsx
lukevella
force-pushed
the
ral-1125-event-card-space-branding
branch
from
c1dec19 to
4974216
Compare
lukevella
force-pushed
the
ral-1125-event-card-space-branding
branch
from
4974216 to
aaf039c
Compare
lukevella
force-pushed
the
ral-1125-event-card-space-branding
branch
from
b1863cb to
3f7d493
Compare
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
1 participant
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.
Summary by CodeRabbit
New Features
Style
Localization
Bug Fixes
Tests