Modernize Biviant web app UI with new design system - #10
Merged
Conversation
Header, Landing, Feed, Event Detail, Dashboard, Auth, Bookmarks, and other components updated Co-authored-by: Flavius Cojocaru <102308258+flvvius@users.noreply.github.com>
Co-authored-by: Flavius Cojocaru <102308258+flvvius@users.noreply.github.com>
Contributor
WalkthroughComprehensive UI redesign across web components and routes, introducing card-based layouts, gradient styling, improved accessibility attributes (ARIA meter), enhanced interaction feedback (animations, loading spinners), and restructured navigation with icon integration and mobile sheet menus. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 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 docstrings
🧪 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 |
![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: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/routes/index.tsx (1)
102-196: 🧹 Nitpick | 🔵 TrivialDefault
WaitlistFormbranch is currently dead code; consolidate or remove.
WaitlistFormis module-local and both call sites in this file (lines 309 and 526–530) passvariant="hero", so the entirevariant === "default"branch is unreachable. The two branches are also ~95% structurally identical (same inputs, same submit, same status paragraph), differing only in spacing/sizing classes. Either drop the unused branch or unify the two variants behind a small style-token map to prevent drift in the duplicated submit/status logic.♻️ Proposed shape (single render path with style tokens)
+const VARIANT_STYLES = { + default: { + wrapper: "flex flex-col gap-3", + row: "flex gap-2", + input: "flex-1", + nameInput: "flex-1", + button: "", + statusMargin: "mt-2", + showArrow: false, + }, + hero: { + wrapper: "flex flex-col gap-4", + row: "flex flex-col sm:flex-row gap-3", + input: "flex-1 h-12 px-4 text-base bg-card border-border", + nameInput: "h-12 px-4 text-base bg-card border-border", + button: "h-12 px-6 text-base font-semibold gap-2 group", + statusMargin: "mt-3", + showArrow: true, + }, +} as const;Then render once with
const v = VARIANT_STYLES[variant]and conditionally include the<ArrowRight />based onv.showArrow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/index.tsx` around lines 102 - 196, The WaitlistForm component currently contains a dead/unreachable "default" render branch because both call sites pass variant="hero" and the two branches duplicate logic; consolidate into a single render path by extracting visual tokens for spacing/sizing into a small VARIANT_STYLES map keyed by variant, then compute const v = VARIANT_STYLES[variant] and use v.* class strings for Input/Button/container classes; render the shared JSX once (use handleSubmit, isPending, message, status as before) and conditionally include the ArrowRight icon when v.showArrow is true to preserve the hero-only arrow behavior.
🤖 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/feed/articles-list.tsx`:
- Around line 56-69: The <img> rendered when article.imageUrl is present lacks
intrinsic size hints; update the <img> in articles-list (the element using
article.imageUrl / article.imageAlt) to include explicit width and height
attributes that match the parent aspect-[4/3] ratio (for example 4:3 like
800x600 or any proportional pair) while keeping loading="lazy" and the alt
fallback, and apply the same change to the analogous <img> in event-card.tsx so
the browser can pre-allocate layout correctly.
- Around line 111-115: The guard and the rendered fallback are using different
operators: the visibility check uses (article.summary || article.rssSnippet)
while the output uses article.summary ?? article.rssSnippet, causing an
empty-string summary to show an empty paragraph. Make them consistent by using
the same operator for both places—either change the guard to (article.summary ??
article.rssSnippet) or (preferably if you want empty strings to fall back)
change the rendered expression to article.summary || article.rssSnippet—so both
the conditional and the displayed value use the same truthiness/nullish
semantics for article.summary and article.rssSnippet.
- Around line 7-26: The Article type declares publishedAt as number but the
backend schema (and getEventBySlug which spreads raw article objects) provides a
string; either change Article.publishedAt to string or convert the value to a
number when mapping in getEventBySlug (or a dedicated normalizeArticle function)
so runtime consumers and sort/arithmetics are correct—also update usage sites
such as the date formatting call (new
Date(article.publishedAt).toLocaleDateString()) to match the chosen type.
Additionally, make the summary fallback operator consistent: replace the
conditional check (article.summary || article.rssSnippet) with a nullish check
(article.summary ?? article.rssSnippet) so an empty-string summary falls back to
rssSnippet, and ensure the rendered expression uses the same operator.
In `@apps/web/src/components/feed/event-card.tsx`:
- Around line 70-75: The image tag in EventCard uses lazy loading but lacks
intrinsic size attributes, causing potential CLS; update the <img> rendering
(the element using event.imageUrl and event.title) to include explicit width and
height attributes (e.g., width={event.imageWidth} height={event.imageHeight}) or
a computed intrinsic ratio (derived from event.imageWidth/event.imageHeight) so
the browser knows the aspect ratio before CSS loads; if event metadata is
unavailable, provide conservative fallback width/height values that match the
parent aspect-[…] container to prevent layout shifts while keeping
loading="lazy" and the existing classes.
- Around line 85-97: The topic chips are rendered using the interactive Button
component inside a Link, which is invalid HTML; replace the Button usage in the
topics mapping (where topics.slice(0, isFeature ? 3 : 2).map(...) is used) with
a non-interactive element (e.g., a <span> or your Badge component) and remove
button-specific props (type, variant, size) and pointer-events-none; preserve
the visual classes (h-7 rounded-full border-border/80 bg-background/70 px-3
text-xs) and the key={topic}, and if needed add a semantic, non-interactive
accessibility attribute (e.g., role="text" or aria-hidden as appropriate) so the
chips remain purely presentational and focusable/tabstop issues are resolved.
- Around line 99-113: The BookmarkButton and topic chips are currently rendered
inside the clickable Card link, producing invalid nested interactive elements;
change the Card wrapper so it is not an anchor and instead render a
full-coverage absolute <Link> overlay with style position:absolute; inset:0 and
aria-label={event.title} (so it handles navigation), then render BookmarkButton,
CardTitle and topic chips as siblings above the overlay with classNames like
relative z-10 (or similar) to keep them interactive and accessible; ensure you
remove the BookmarkButton from inside the Link and adjust any onClick handlers
that called e.preventDefault() to rely on the overlay link for card navigation.
In `@apps/web/src/components/header.tsx`:
- Line 40: The current active-check uses currentPath.startsWith(to) which can
false-positive match siblings; replace that logic with a small helper (e.g.,
isNavActive or isActivePath) that returns currentPath === to ||
currentPath.startsWith(`${to}/`) (and keep the special root case when to ===
"/"), and use that helper for both desktop and mobile navs to avoid duplication
and drift; alternatively, swap to TanStack Router's Link with activeOptions or
useMatchRoute if preferred.
In `@apps/web/src/components/sign-in-form.tsx`:
- Around line 87-91: The validation error messages rendered from
field.state.meta.errors in the sign-in-form component are missing an ARIA live
region; update the error rendering (the JSX that maps field.state.meta.errors in
apps/web/src/components/sign-in-form.tsx) to include an accessible live region
by adding role="alert" and aria-live="polite" (or similar) to the element that
wraps each error message so screen readers announce dynamic errors; apply the
same change to the other error block in this file (the second mapping at lines
~112–116) to ensure both error blocks are announced.
- Around line 153-183: The Google sign-in onClick handler currently fires
authClient.signIn.social(...) without handling errors and the SVG lacks
accessibility annotation; update the Button onClick to await the promise from
authClient.signIn.social (or attach a .catch) and surface failures via the same
onError/toast path used by the email flow (so users see errors), and add
aria-hidden="true" to the inline SVG element to hide the decorative icon from
assistive tech; adjust the handler in the sign-in form Button that calls
authClient.signIn.social to perform error handling and user feedback.
- Around line 129-136: The submit button's loading UI (state.isSubmitting,
Loader2) is not accessible: mark the spinner (<Loader2>) aria-hidden="true", set
aria-busy={state.isSubmitting} on the button element, and move the changing text
("Sign In" / "Signing in...") into a small element with aria-live="polite" so
screen readers announce the status change; update the button markup in the
sign-in form component to use these attributes while preserving the existing
visuals.
In `@apps/web/src/components/sign-up-form.tsx`:
- Around line 89-93: The validation error paragraphs rendered from
field.state.meta.errors (the map rendering at lines showing
{field.state.meta.errors.map(...)} in sign-up-form.tsx) are not announced to
screen readers; update each error block to include an ARIA live region (e.g.,
add role="alert" and/or aria-live="assertive" on the <p> elements) and also mark
the corresponding <Input> with aria-invalid={field.state.meta.errors.length > 0}
and aria-describedby pointing to the error element id so assistive tech is
notified and the error is associated with the field; ensure each error <p> has a
stable id (e.g., `${field.name}-error`) and apply the same change to all three
field error blocks.
- Around line 156-163: The submit button in sign-up-form.tsx flips its label
silently and the Loader2 SVG is announced; when state.isSubmitting is true, make
the label change accessible by adding a polite live region (e.g., wrap the
changing text in an element with aria-live="polite") or set aria-busy="true" on
the button (on the element that triggers the change), and mark the Loader2
spinner as decorative (e.g., add aria-hidden="true" or equivalent) so it is not
announced by screen readers; update the JSX around state.isSubmitting, Loader2,
and the button to implement these ARIA attributes and ensure the spinner is
non-focusable.
- Around line 180-210: The Google social sign-in call in the Button onClick uses
authClient.signIn.social(...) with no error handling and the SVG lacks
aria-hidden; wrap the sign-in call in an async handler or add a Promise catch to
handle failures and call the existing toast.error(...) (same pattern as the
email flow's onError) so auth/network errors surface to the user, and add
aria-hidden="true" to the SVG inside the Button to improve accessibility; apply
the identical changes to the Google sign-in Button in sign-in-form.tsx (same
authClient.signIn.social usage and SVG).
In `@apps/web/src/components/user-menu.tsx`:
- Around line 39-46: Add aria-hidden="true" to decorative elements so screen
readers don't announce them: mark the avatar initial container (the div
rendering {userInitial}) with aria-hidden="true", add aria-hidden="true" to the
<ChevronDown /> component instance, and also apply aria-hidden="true" to the
<User /> and <LogOut /> icon components used inside the DropdownMenuItem entries
(these are the decorative icons adjacent to the menu text).
In `@apps/web/src/routes/bookmarks.tsx`:
- Around line 55-59: Replace the invalid nested interactive markup where a
TanStack Router Link (<Link>) wraps a real button (<Button>) by making the
Button the parent with asChild and placing the Link inside it so the Link
becomes the button element; find the occurrences using the symbols Link and
Button (notably in the bookmarks component instances around the Sign in to
continue and other buttons, unsubscribe and event.$slug usages) and change the
pattern from <Link><Button>...</Button></Link> to <Button asChild><Link
to="...">...</Link></Button>, keeping existing props like size and className on
Button and preserving Link's to prop and children.
In `@apps/web/src/routes/dashboard.tsx`:
- Around line 113-162: The three dashboard stat cards currently render hardcoded
"0"/"0 days" which can mislead returning users; fetch the real bookmarks count
via api.interactions.getBookmarkedEvents (call it in the route data loader or
the component's data hook) and render its length inside the Bookmarks
CardContent instead of the literal "0", and change the Reading Streak and
Articles Read CardContent to display a "Coming soon" badge/text (or remove those
Cards) until their metrics (streak and weekly read count) are implemented;
update the Bookmarks rendering in the Card/CardContent that currently shows "0"
and adjust Reading Streak and Articles Read in their CardContent/CardTitle.
- Line 86: Authenticated container's root div (the element with className
"bg-gradient-to-b from-background via-background to-muted/35") is missing the
same min-height used by the unauthenticated/loading branches; update that
className to include min-h-[calc(100vh-4rem)] so the authenticated view matches
the other branches (the existing unauthenticated/loading containers using
min-h-[calc(100vh-4rem)] around lines where the unauthenticated and loading JSX
render). Ensure you only add the min-h utility to the authenticated container's
className so page height and footer alignment remain consistent.
- Around line 117-119: Add aria-hidden="true" to the decorative inline SVG
elements so screen readers ignore them: locate the SVG tags (e.g., the <svg
className="size-4" ...> instances used in the stats cards and Quick Actions
cards in apps/web/src/routes/dashboard.tsx and the inline SVGs in
apps/web/src/routes/bookmarks.tsx) and add aria-hidden="true" to each SVG
element; keep the existing attributes (className, viewBox, stroke, etc.)
unchanged and do not remove any accessible text labels that accompany the cards.
- Around line 50-58: Add ARIA live attributes to the loading panels so screen
readers announce status changes: update the Loading... container div (the
rounded card div with classes "rounded-[1.2rem] border border-border/70
bg-card/70 px-6 py-8 text-sm text-muted-foreground" in
apps/web/src/routes/dashboard.tsx) to include role="status" and
aria-live="polite"; apply the same change to the auth-loading panel and the
currentUser===undefined loading panel in this file and to the analogous
Loading... / "Loading bookmarks..." panels in apps/web/src/routes/feed.tsx and
apps/web/src/routes/bookmarks.tsx so they all use role="status" and
aria-live="polite".
In `@apps/web/src/routes/event`.$slug.tsx:
- Around line 200-211: Replace the hard-to-read inline cast lookup for grid
column classes with a small, named lookup map constant and use it in the
TabsList className; specifically, create a const (e.g., gridColsMap = {1:
"grid-cols-1", 2: "grid-cols-2", 3: "grid-cols-3"}) and replace the expression
`({ 1: ..., 2: ..., 3: ... } as Record<number, string>)[tabCount] ??
"grid-cols-3"` with `gridColsMap[tabCount]` when setting TabsList's className;
remove the unnecessary `?? "grid-cols-3"` fallback since tabCount is guaranteed
1–3 (TabsList, tabCount, TabsTrigger are the relevant symbols).
- Around line 153-180: The current unique-source extraction using
articles.map(...).filter(...) is O(n²) and forces non-null assertions (source!)
downstream; replace that with a single-pass Map keyed on source._id to
deduplicate and produce an array of non-null sources before slicing and
rendering. Concretely, iterate articles and for each article.source (guarding
null/undefined) set map.set(source._id, source), then use
Array.from(map.values()) (or [...map.values()]) to get a deduped, typed array of
sources, slice(0,5) and map over that array in the JSX so you can remove the
arr.findIndex logic and the source! assertions (ensuring you reference articles,
source, and _id exactly as in the diff).
In `@apps/web/src/routes/feed.tsx`:
- Around line 86-91: The H1 currently contains marketing copy rather than a
concise page title; change the <h1> in the feed route (the element rendering the
current headline in apps/web/src/routes/feed.tsx) to a short semantic title like
"News Feed" and move the existing marketing sentence ("See the day's biggest
stories with the image front and center.") into the adjacent <p> (or update the
<p> content to include that marketing sentence), preserving existing className
styling (text-4xl font-bold... and max-w-[55ch] text-sm...) so visual layout
stays the same while improving semantics/SEO.
---
Outside diff comments:
In `@apps/web/src/routes/index.tsx`:
- Around line 102-196: The WaitlistForm component currently contains a
dead/unreachable "default" render branch because both call sites pass
variant="hero" and the two branches duplicate logic; consolidate into a single
render path by extracting visual tokens for spacing/sizing into a small
VARIANT_STYLES map keyed by variant, then compute const v =
VARIANT_STYLES[variant] and use v.* class strings for Input/Button/container
classes; render the shared JSX once (use handleSubmit, isPending, message,
status as before) and conditionally include the ArrowRight icon when v.showArrow
is true to preserve the hero-only arrow behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c0f295a6-91f4-4c24-a354-0733b09e3429
📒 Files selected for processing (15)
apps/web/src/components/bias-indicator.tsxapps/web/src/components/bookmark-button.tsxapps/web/src/components/feed/articles-list.tsxapps/web/src/components/feed/event-card.tsxapps/web/src/components/header.tsxapps/web/src/components/sign-in-form.tsxapps/web/src/components/sign-up-form.tsxapps/web/src/components/user-menu.tsxapps/web/src/routes/__root.tsxapps/web/src/routes/bookmarks.tsxapps/web/src/routes/dashboard.tsxapps/web/src/routes/event.$slug.tsxapps/web/src/routes/feed.tsxapps/web/src/routes/index.tsxapps/web/src/routes/unsubscribe.tsx
Comment on lines
7
to
26
| type Article = { | ||
| _id: Id<"articles">; | ||
| title: string; | ||
| summary: string; | ||
| summary?: string; | ||
| rssSnippet?: string; | ||
| imageUrl?: string; | ||
| imageAlt?: string; | ||
| canonicalUrl: string; | ||
| publishedAt: string; | ||
| publishedAt: number; | ||
| source: { | ||
| _id: Id<"sources">; | ||
| name: string; | ||
| logoUrl: string; | ||
| logoUrl?: string; | ||
| baseBias: number; | ||
| reliabilityScore: number; | ||
| mbfcCategory?: string; | ||
| mbfcFactual?: string; | ||
| mbfcCredibility?: string; | ||
| } | null; | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm schema and check whether any frontend code treats publishedAt as a number.
fd -t f schema.ts packages/backend/convex
rg -nP -C2 '\bpublishedAt\s*:\s*v\.(string|number)' packages/backend
echo "--- frontend usages ---"
rg -nP --type=tsx --type=ts -C2 '\bpublishedAt\b' apps/web/srcRepository: flvvius/news
Length of output: 452
🏁 Script executed:
# Read the articles-list.tsx file to see the actual type declaration
cat -n apps/web/src/components/feed/articles-list.tsx | head -30Repository: flvvius/news
Length of output: 1051
🏁 Script executed:
# Search for publishedAt usage more broadly in the frontend
rg -n "publishedAt" apps/web/src --type tsRepository: flvvius/news
Length of output: 255
🏁 Script executed:
# Check the backend schema for all fields mentioned: rssSnippet, imageUrl, imageAlt, mbfc*
cat -n packages/backend/convex/schema.ts | grep -A 50 "articles:"Repository: flvvius/news
Length of output: 1946
🏁 Script executed:
# Check the line context around 102 to see the summary/rssSnippet rendering logic
cat -n apps/web/src/components/feed/articles-list.tsx | sed -n '95,110p'Repository: flvvius/news
Length of output: 784
🏁 Script executed:
# Search for any numeric operations on publishedAt
rg -n "publishedAt\s*[-+*/<>]|Date\.now\(\).*publishedAt|publishedAt.*Date\.now" apps/web/srcRepository: flvvius/news
Length of output: 38
🏁 Script executed:
# Check the query/loader that fetches articles to see data transformation
fd -t f "events.ts" packages/backend/convexRepository: flvvius/news
Length of output: 90
🏁 Script executed:
# Read the events.ts query to see how article data is returned
cat -n packages/backend/convex/events.ts | grep -A 30 "getEventBySlug\|article"Repository: flvvius/news
Length of output: 2037
🏁 Script executed:
# Check the schema for the optional fields: rssSnippet, imageUrl, imageAlt
rg -n "rssSnippet|imageUrl|imageAlt|mbfcCategory|mbfcFactual|mbfcCredibility" packages/backend/convex/schema.tsRepository: flvvius/news
Length of output: 97
🏁 Script executed:
# Search more thoroughly for imageAlt and the mbfc fields in schema
rg -n "imageAlt|mbfc" packages/backend/convex/schema.tsRepository: flvvius/news
Length of output: 38
🏁 Script executed:
# Check if there's any transformation or data mapping happening elsewhere for articles
rg -n "publishedAt.*parse\|publishedAt.*Date\|Date\.parse" packages/backend/convex --type tsRepository: flvvius/news
Length of output: 38
🏁 Script executed:
# Verify the sources schema to see if mbfc fields are on sources
cat -n packages/backend/convex/schema.ts | grep -B 5 -A 30 "sources:"Repository: flvvius/news
Length of output: 1614
🏁 Script executed:
# Search for the summary rendering logic to check || vs ?? inconsistency
cat -n apps/web/src/components/feed/articles-list.tsx | sed -n '110,125p'Repository: flvvius/news
Length of output: 845
🏁 Script executed:
# Look for all places where summary or rssSnippet are rendered
rg -n "article\.summary|article\.rssSnippet" apps/web/src/components/feed/articles-list.tsxRepository: flvvius/news
Length of output: 194
publishedAt type mismatch: schema stores string, frontend type declares number.
Confirmed: packages/backend/convex/schema.ts defines publishedAt: v.string(), and getEventBySlug spreads the raw article ({ ...article, source }) without conversion. The frontend type at line 15 declares publishedAt: number, which contradicts the data shape. new Date(article.publishedAt).toLocaleDateString() at line 102 works at runtime because Date() accepts both numbers and strings, but the type lie is a type safety violation—any consumer attempting numeric operations (arithmetic, sorting) will silently misbehave.
Secondary issue: || vs ?? inconsistency for summary fallback (lines 111–113).
Line 111 uses (article.summary || article.rssSnippet) && (falsy check), but line 113 renders article.summary ?? article.rssSnippet (null/undefined check). If summary is an empty string "", the condition is truthy and renders, but the actual output will be empty instead of falling back to rssSnippet. Use the same operator in both places.
Note on missing schema fields:
rssSnippet, imageUrl (on articles), imageAlt, and source.mbfcCategory/mbfcFactual/mbfcCredibility are not in the Convex schema. They're typed optional in the frontend, so rendering is safe (they'll be undefined), but they'll never populate until the backend schema adds them.
Action: Change publishedAt: number to publishedAt: string, or normalize at the data layer if numeric timestamps are intended. Also align the summary/rssSnippet operators.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/feed/articles-list.tsx` around lines 7 - 26, The
Article type declares publishedAt as number but the backend schema (and
getEventBySlug which spreads raw article objects) provides a string; either
change Article.publishedAt to string or convert the value to a number when
mapping in getEventBySlug (or a dedicated normalizeArticle function) so runtime
consumers and sort/arithmetics are correct—also update usage sites such as the
date formatting call (new Date(article.publishedAt).toLocaleDateString()) to
match the chosen type. Additionally, make the summary fallback operator
consistent: replace the conditional check (article.summary ||
article.rssSnippet) with a nullish check (article.summary ?? article.rssSnippet)
so an empty-string summary falls back to rssSnippet, and ensure the rendered
expression uses the same operator.
Comment on lines
+56
to
69
| {article.imageUrl ? ( | ||
| <img | ||
| src={article.imageUrl} | ||
| alt={article.imageAlt ?? article.title} | ||
| className="h-full w-full object-cover" | ||
| loading="lazy" | ||
| /> | ||
| ) : ( | ||
| <div className="flex h-full items-center justify-center bg-gradient-to-br from-muted to-background"> | ||
| <span className="text-xs font-medium uppercase tracking-[0.18em] text-muted-foreground"> | ||
| {article.source?.name ?? "Source"} | ||
| </span> | ||
| <BiasIndicator | ||
| bias={article.source.baseBias} | ||
| size="sm" | ||
| thresholds={thresholds} | ||
| /> | ||
| </> | ||
| </div> | ||
| )} |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Decorative placeholder image lacks dimensions; consider lazy-loading the real <img> similarly.
The conditional <img> uses loading="lazy" (good), but no width/height attributes. Inside the aspect-[4/3] parent the layout is already reserved, so this is minor — but adding intrinsic size hints helps the browser pre-allocate ratio when CSS is deferred. Same note as event-card.tsx.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/feed/articles-list.tsx` around lines 56 - 69, The
<img> rendered when article.imageUrl is present lacks intrinsic size hints;
update the <img> in articles-list (the element using article.imageUrl /
article.imageAlt) to include explicit width and height attributes that match the
parent aspect-[4/3] ratio (for example 4:3 like 800x600 or any proportional
pair) while keeping loading="lazy" and the alt fallback, and apply the same
change to the analogous <img> in event-card.tsx so the browser can pre-allocate
layout correctly.
Comment on lines
+111
to
+115
| {(article.summary || article.rssSnippet) && ( | ||
| <p className="max-w-[65ch] text-sm text-muted-foreground"> | ||
| {article.summary ?? article.rssSnippet} | ||
| </p> | ||
| )} |
Contributor
There was a problem hiding this comment.
Inconsistent || vs ?? between the gate and the rendered fallback.
The visibility check uses (article.summary || article.rssSnippet) (truthy), but the rendered text uses article.summary ?? article.rssSnippet (nullish only). For an empty-string summary the gate would render the <p> with the rendered expression resolving to "" (because ?? doesn't fall through empty strings). Use the same operator on both sides.
🩹 Suggested fix
- {(article.summary || article.rssSnippet) && (
- <p className="max-w-[65ch] text-sm text-muted-foreground">
- {article.summary ?? article.rssSnippet}
- </p>
- )}
+ {(article.summary || article.rssSnippet) && (
+ <p className="max-w-[65ch] text-sm text-muted-foreground">
+ {article.summary || article.rssSnippet}
+ </p>
+ )}📝 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.
Suggested change
| {(article.summary || article.rssSnippet) && ( | |
| <p className="max-w-[65ch] text-sm text-muted-foreground"> | |
| {article.summary ?? article.rssSnippet} | |
| </p> | |
| )} | |
| {(article.summary || article.rssSnippet) && ( | |
| <p className="max-w-[65ch] text-sm text-muted-foreground"> | |
| {article.summary || article.rssSnippet} | |
| </p> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/feed/articles-list.tsx` around lines 111 - 115, The
guard and the rendered fallback are using different operators: the visibility
check uses (article.summary || article.rssSnippet) while the output uses
article.summary ?? article.rssSnippet, causing an empty-string summary to show
an empty paragraph. Make them consistent by using the same operator for both
places—either change the guard to (article.summary ?? article.rssSnippet) or
(preferably if you want empty strings to fall back) change the rendered
expression to article.summary || article.rssSnippet—so both the conditional and
the displayed value use the same truthiness/nullish semantics for
article.summary and article.rssSnippet.
Comment on lines
+70
to
+75
| <img | ||
| src={event.imageUrl} | ||
| alt={event.title} | ||
| className="h-full w-full object-cover transition-transform duration-500 group-hover:scale-[1.02]" | ||
| loading="lazy" | ||
| /> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add explicit width/height (or aspect-ratio) on <img> to avoid CLS during lazy load.
The card image uses loading="lazy" and lives inside an aspect-[…] parent so the layout is already reserved by CSS, but providing width/height attributes (or using next/image-style intrinsic sizing) helps the browser compute intrinsic ratio earlier and prevents layout shifts when CSS is delayed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/feed/event-card.tsx` around lines 70 - 75, The image
tag in EventCard uses lazy loading but lacks intrinsic size attributes, causing
potential CLS; update the <img> rendering (the element using event.imageUrl and
event.title) to include explicit width and height attributes (e.g.,
width={event.imageWidth} height={event.imageHeight}) or a computed intrinsic
ratio (derived from event.imageWidth/event.imageHeight) so the browser knows the
aspect ratio before CSS loads; if event metadata is unavailable, provide
conservative fallback width/height values that match the parent aspect-[…]
container to prevent layout shifts while keeping loading="lazy" and the existing
classes.
Comment on lines
85
to
+97
| <div className="flex flex-wrap items-center gap-2"> | ||
| {topics.length > 0 ? ( | ||
| topics.map((topic) => ( | ||
| <Button | ||
| key={topic} | ||
| type="button" | ||
| variant="outline" | ||
| size="sm" | ||
| className="h-6 rounded-full px-2 text-xs" | ||
| > | ||
| {topic} | ||
| </Button> | ||
| )) | ||
| ) : ( | ||
| {(topics.length > 0 ? topics : ["General"]).slice(0, isFeature ? 3 : 2).map((topic) => ( | ||
| <Button | ||
| key={topic} | ||
| type="button" | ||
| variant="outline" | ||
| size="sm" | ||
| className="h-6 rounded-full px-2 text-xs" | ||
| className="h-7 rounded-full border-border/80 bg-background/70 px-3 text-xs pointer-events-none" | ||
| > | ||
| General | ||
| {topic} | ||
| </Button> | ||
| )} | ||
| ))} | ||
| </div> |
Contributor
There was a problem hiding this comment.
Invalid HTML: interactive <Button> topic chips nested inside <Link> (<a>).
<button> inside <a> is invalid per the HTML spec (no interactive content inside <a>) and produces nested-tabstop / focus issues for keyboard and screen-reader users. pointer-events-none only blocks pointer events; the buttons remain focusable and the DOM tree is still invalid. Since these are visual labels (not actions), use a non-interactive element such as a <span> or a Badge component.
🩹 Suggested fix
- {(topics.length > 0 ? topics : ["General"]).slice(0, isFeature ? 3 : 2).map((topic) => (
- <Button
- key={topic}
- type="button"
- variant="outline"
- size="sm"
- className="h-7 rounded-full border-border/80 bg-background/70 px-3 text-xs pointer-events-none"
- >
- {topic}
- </Button>
- ))}
+ {(topics.length > 0 ? topics : ["General"])
+ .slice(0, isFeature ? 3 : 2)
+ .map((topic) => (
+ <span
+ key={topic}
+ className="inline-flex h-7 items-center rounded-full border border-border/80 bg-background/70 px-3 text-xs"
+ >
+ {topic}
+ </span>
+ ))}📝 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.
Suggested change
| <div className="flex flex-wrap items-center gap-2"> | |
| {topics.length > 0 ? ( | |
| topics.map((topic) => ( | |
| <Button | |
| key={topic} | |
| type="button" | |
| variant="outline" | |
| size="sm" | |
| className="h-6 rounded-full px-2 text-xs" | |
| > | |
| {topic} | |
| </Button> | |
| )) | |
| ) : ( | |
| {(topics.length > 0 ? topics : ["General"]).slice(0, isFeature ? 3 : 2).map((topic) => ( | |
| <Button | |
| key={topic} | |
| type="button" | |
| variant="outline" | |
| size="sm" | |
| className="h-6 rounded-full px-2 text-xs" | |
| className="h-7 rounded-full border-border/80 bg-background/70 px-3 text-xs pointer-events-none" | |
| > | |
| General | |
| {topic} | |
| </Button> | |
| )} | |
| ))} | |
| </div> | |
| <div className="flex flex-wrap items-center gap-2"> | |
| {(topics.length > 0 ? topics : ["General"]) | |
| .slice(0, isFeature ? 3 : 2) | |
| .map((topic) => ( | |
| <span | |
| key={topic} | |
| className="inline-flex h-7 items-center rounded-full border border-border/80 bg-background/70 px-3 text-xs" | |
| > | |
| {topic} | |
| </span> | |
| ))} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/feed/event-card.tsx` around lines 85 - 97, The topic
chips are rendered using the interactive Button component inside a Link, which
is invalid HTML; replace the Button usage in the topics mapping (where
topics.slice(0, isFeature ? 3 : 2).map(...) is used) with a non-interactive
element (e.g., a <span> or your Badge component) and remove button-specific
props (type, variant, size) and pointer-events-none; preserve the visual classes
(h-7 rounded-full border-border/80 bg-background/70 px-3 text-xs) and the
key={topic}, and if needed add a semantic, non-interactive accessibility
attribute (e.g., role="text" or aria-hidden as appropriate) so the chips remain
purely presentational and focusable/tabstop issues are resolved.
Comment on lines
+113
to
+162
| <div className="grid gap-4 sm:grid-cols-3"> | ||
| <Card className="overflow-hidden rounded-[1.2rem] border-border/70 bg-card/80 py-0"> | ||
| <CardHeader className="border-b border-border/70 bg-muted/30 py-4 px-5"> | ||
| <CardTitle className="text-sm font-medium text-muted-foreground flex items-center gap-2"> | ||
| <svg className="size-4" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={1.5}> | ||
| <path strokeLinecap="round" strokeLinejoin="round" d="M2.25 18L9 11.25l4.306 4.307a11.95 11.95 0 015.814-5.519l2.74-1.22m0 0l-5.94-2.28m5.94 2.28l-2.28 5.941" /> | ||
| </svg> | ||
| Reading Streak | ||
| </CardTitle> | ||
| </CardHeader> | ||
| <CardContent className="px-5 py-5"> | ||
| <div className="text-3xl font-bold">0 days</div> | ||
| <p className="text-xs text-muted-foreground mt-1"> | ||
| Start reading to build your streak | ||
| </p> | ||
| </CardContent> | ||
| </Card> | ||
|
|
||
| <Card className="overflow-hidden rounded-[1.2rem] border-border/70 bg-card/80 py-0"> | ||
| <CardHeader className="border-b border-border/70 bg-muted/30 py-4 px-5"> | ||
| <CardTitle className="text-sm font-medium text-muted-foreground flex items-center gap-2"> | ||
| <svg className="size-4" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={1.5}> | ||
| <path strokeLinecap="round" strokeLinejoin="round" d="M17.593 3.322c1.1.128 1.907 1.077 1.907 2.185V21L12 17.25 4.5 21V5.507c0-1.108.806-2.057 1.907-2.185a48.507 48.507 0 0111.186 0z" /> | ||
| </svg> | ||
| Bookmarks | ||
| </CardTitle> | ||
| </CardHeader> | ||
| <CardContent className="px-5 py-5"> | ||
| <div className="text-3xl font-bold">0</div> | ||
| <p className="text-xs text-muted-foreground mt-1"> | ||
| Saved for later | ||
| </p> | ||
| </CardContent> | ||
| </Card> | ||
|
|
||
| <Card className="overflow-hidden rounded-[1.2rem] border-border/70 bg-card/80 py-0"> | ||
| <CardHeader className="border-b border-border/70 bg-muted/30 py-4 px-5"> | ||
| <CardTitle className="text-sm font-medium text-muted-foreground flex items-center gap-2"> | ||
| <svg className="size-4" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={1.5}> | ||
| <path strokeLinecap="round" strokeLinejoin="round" d="M12 7.5h1.5m-1.5 3h1.5m-7.5 3h7.5m-7.5 3h7.5m3-9h3.375c.621 0 1.125.504 1.125 1.125V18a2.25 2.25 0 01-2.25 2.25M16.5 7.5V18a2.25 2.25 0 002.25 2.25M16.5 7.5V4.875c0-.621-.504-1.125-1.125-1.125H4.125C3.504 3.75 3 4.254 3 4.875V18a2.25 2.25 0 002.25 2.25h13.5M6 7.5h3v3H6v-3z" /> | ||
| </svg> | ||
| Articles Read | ||
| </CardTitle> | ||
| </CardHeader> | ||
| <CardContent className="px-5 py-5"> | ||
| <div className="text-3xl font-bold">0</div> | ||
| <p className="text-xs text-muted-foreground mt-1">This week</p> | ||
| </CardContent> | ||
| </Card> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Hardcoded 0 stats cards risk being misleading to users.
Reading Streak, Bookmarks, and Articles Read are rendered with literal 0/0 days regardless of the user's actual activity (the user does have a real bookmarks count via api.interactions.getBookmarkedEvents). Until these are wired to real metrics, consider either marking them as "Coming soon" (consistent with the Profile/Settings cards below) or removing them, so a returning user with bookmarks doesn't see "Bookmarks: 0".
Want me to wire Bookmarks to api.interactions.getBookmarkedEvents.length and tag the other two with a "Coming soon" badge?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/dashboard.tsx` around lines 113 - 162, The three
dashboard stat cards currently render hardcoded "0"/"0 days" which can mislead
returning users; fetch the real bookmarks count via
api.interactions.getBookmarkedEvents (call it in the route data loader or the
component's data hook) and render its length inside the Bookmarks CardContent
instead of the literal "0", and change the Reading Streak and Articles Read
CardContent to display a "Coming soon" badge/text (or remove those Cards) until
their metrics (streak and weekly read count) are implemented; update the
Bookmarks rendering in the Card/CardContent that currently shows "0" and adjust
Reading Streak and Articles Read in their CardContent/CardTitle.
Comment on lines
+117
to
+119
| <svg className="size-4" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={1.5}> | ||
| <path strokeLinecap="round" strokeLinejoin="round" d="M2.25 18L9 11.25l4.306 4.307a11.95 11.95 0 015.814-5.519l2.74-1.22m0 0l-5.94-2.28m5.94 2.28l-2.28 5.941" /> | ||
| </svg> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Decorative inline SVG icons should be marked aria-hidden="true".
The inline SVGs in the stats cards and Quick Actions cards (lines 117–119, 134–136, 151–153, 176–178, 194–196, 211–213, 225–228) are decorative — adjacent text already labels each card. Add aria-hidden="true" to each to avoid noisy AT announcements. Same applies to the inline SVGs in apps/web/src/routes/bookmarks.tsx (lines 43–45, 142–144).
♻️ Example
- <svg className="size-4" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={1.5}>
+ <svg aria-hidden="true" className="size-4" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={1.5}>📝 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.
Suggested change
| <svg className="size-4" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={1.5}> | |
| <path strokeLinecap="round" strokeLinejoin="round" d="M2.25 18L9 11.25l4.306 4.307a11.95 11.95 0 015.814-5.519l2.74-1.22m0 0l-5.94-2.28m5.94 2.28l-2.28 5.941" /> | |
| </svg> | |
| <svg aria-hidden="true" className="size-4" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={1.5}> | |
| <path strokeLinecap="round" strokeLinejoin="round" d="M2.25 18L9 11.25l4.306 4.307a11.95 11.95 0 015.814-5.519l2.74-1.22m0 0l-5.94-2.28m5.94 2.28l-2.28 5.941" /> | |
| </svg> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/dashboard.tsx` around lines 117 - 119, Add
aria-hidden="true" to the decorative inline SVG elements so screen readers
ignore them: locate the SVG tags (e.g., the <svg className="size-4" ...>
instances used in the stats cards and Quick Actions cards in
apps/web/src/routes/dashboard.tsx and the inline SVGs in
apps/web/src/routes/bookmarks.tsx) and add aria-hidden="true" to each SVG
element; keep the existing attributes (className, viewBox, stroke, etc.)
unchanged and do not remove any accessible text labels that accompany the cards.
Comment on lines
+153
to
+180
| <div className="flex -space-x-3"> | ||
| {articles | ||
| .map((article) => article.source) | ||
| .filter((source, index, array) => | ||
| source && | ||
| array.findIndex((candidate) => candidate?._id === source._id) === index, | ||
| ) | ||
| .slice(0, 5) | ||
| .map((source) => ( | ||
| <div | ||
| key={source!._id} | ||
| className="flex h-11 w-11 items-center justify-center overflow-hidden rounded-full border-2 border-background bg-background shadow-sm" | ||
| title={source!.name} | ||
| > | ||
| {source?.logoUrl ? ( | ||
| <img | ||
| src={source.logoUrl} | ||
| alt={source.name} | ||
| className="h-full w-full object-contain p-1.5" | ||
| /> | ||
| ) : ( | ||
| <span className="text-xs font-medium text-foreground"> | ||
| {source?.name.charAt(0)} | ||
| </span> | ||
| )} | ||
| </div> | ||
| ))} | ||
| </div> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider a Map-based dedup for unique sources.
The current pattern articles.map(...).filter((source, i, arr) => source && arr.findIndex(c => c?._id === source._id) === i) is O(n²) and requires source! non-null assertions downstream. A Map keyed on _id removes both the quadratic cost and the assertions. Not a correctness issue, just clarity.
♻️ Proposed refactor
- <div className="flex -space-x-3">
- {articles
- .map((article) => article.source)
- .filter((source, index, array) =>
- source &&
- array.findIndex((candidate) => candidate?._id === source._id) === index,
- )
- .slice(0, 5)
- .map((source) => (
- <div
- key={source!._id}
- ...
- title={source!.name}
- >
- {source?.logoUrl ? (
+ <div className="flex -space-x-3">
+ {Array.from(
+ new Map(
+ articles
+ .map((a) => a.source)
+ .filter((s): s is NonNullable<typeof s> => Boolean(s))
+ .map((s) => [s._id, s] as const),
+ ).values(),
+ )
+ .slice(0, 5)
+ .map((source) => (
+ <div
+ key={source._id}
+ ...
+ title={source.name}
+ >
+ {source.logoUrl ? (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/event`.$slug.tsx around lines 153 - 180, The current
unique-source extraction using articles.map(...).filter(...) is O(n²) and forces
non-null assertions (source!) downstream; replace that with a single-pass Map
keyed on source._id to deduplicate and produce an array of non-null sources
before slicing and rendering. Concretely, iterate articles and for each
article.source (guarding null/undefined) set map.set(source._id, source), then
use Array.from(map.values()) (or [...map.values()]) to get a deduped, typed
array of sources, slice(0,5) and map over that array in the JSX so you can
remove the arr.findIndex logic and the source! assertions (ensuring you
reference articles, source, and _id exactly as in the diff).
Comment on lines
+200
to
211
| <Tabs defaultValue="center" className="w-full gap-5"> | ||
| <TabsList | ||
| className={`grid w-full ${({ 1: "grid-cols-1", 2: "grid-cols-2", 3: "grid-cols-3" } as Record<number, string>)[tabCount] ?? "grid-cols-3"}`} | ||
| > | ||
| {event.perspectiveSummaries?.left && ( | ||
| <TabsTrigger value="left">Left</TabsTrigger> | ||
| )} | ||
| <TabsTrigger value="center">Center</TabsTrigger> | ||
| {event.perspectiveSummaries.right && ( | ||
| {event.perspectiveSummaries?.right && ( | ||
| <TabsTrigger value="right">Right</TabsTrigger> | ||
| )} | ||
| </TabsList> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Simplify the dynamic grid-cols-* selection.
The inline object cast ({ 1: ..., 2: ..., 3: ... } as Record<number, string>)[tabCount] ?? "grid-cols-3" is harder to read than necessary. Since tabCount is always between 1 and 3 (center is unconditional), a small lookup with a const map is clearer; the ?? "grid-cols-3" fallback is also dead today.
♻️ Proposed refactor
+const TAB_GRID_COLS: Record<1 | 2 | 3, string> = {
+ 1: "grid-cols-1",
+ 2: "grid-cols-2",
+ 3: "grid-cols-3",
+};
+
...
- <TabsList
- className={`grid w-full ${({ 1: "grid-cols-1", 2: "grid-cols-2", 3: "grid-cols-3" } as Record<number, string>)[tabCount] ?? "grid-cols-3"}`}
- >
+ <TabsList
+ className={`grid w-full ${TAB_GRID_COLS[tabCount as 1 | 2 | 3]}`}
+ >📝 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.
Suggested change
| <Tabs defaultValue="center" className="w-full gap-5"> | |
| <TabsList | |
| className={`grid w-full ${({ 1: "grid-cols-1", 2: "grid-cols-2", 3: "grid-cols-3" } as Record<number, string>)[tabCount] ?? "grid-cols-3"}`} | |
| > | |
| {event.perspectiveSummaries?.left && ( | |
| <TabsTrigger value="left">Left</TabsTrigger> | |
| )} | |
| <TabsTrigger value="center">Center</TabsTrigger> | |
| {event.perspectiveSummaries.right && ( | |
| {event.perspectiveSummaries?.right && ( | |
| <TabsTrigger value="right">Right</TabsTrigger> | |
| )} | |
| </TabsList> | |
| const TAB_GRID_COLS: Record<1 | 2 | 3, string> = { | |
| 1: "grid-cols-1", | |
| 2: "grid-cols-2", | |
| 3: "grid-cols-3", | |
| }; | |
| <Tabs defaultValue="center" className="w-full gap-5"> | |
| <TabsList | |
| className={`grid w-full ${TAB_GRID_COLS[tabCount as 1 | 2 | 3]}`} | |
| > | |
| {event.perspectiveSummaries?.left && ( | |
| <TabsTrigger value="left">Left</TabsTrigger> | |
| )} | |
| <TabsTrigger value="center">Center</TabsTrigger> | |
| {event.perspectiveSummaries?.right && ( | |
| <TabsTrigger value="right">Right</TabsTrigger> | |
| )} | |
| </TabsList> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/event`.$slug.tsx around lines 200 - 211, Replace the
hard-to-read inline cast lookup for grid column classes with a small, named
lookup map constant and use it in the TabsList className; specifically, create a
const (e.g., gridColsMap = {1: "grid-cols-1", 2: "grid-cols-2", 3:
"grid-cols-3"}) and replace the expression `({ 1: ..., 2: ..., 3: ... } as
Record<number, string>)[tabCount] ?? "grid-cols-3"` with `gridColsMap[tabCount]`
when setting TabsList's className; remove the unnecessary `?? "grid-cols-3"`
fallback since tabCount is guaranteed 1–3 (TabsList, tabCount, TabsTrigger are
the relevant symbols).
Comment on lines
+86
to
+91
| <h1 className="text-4xl font-bold tracking-tight text-foreground sm:text-5xl text-balance"> | ||
| See the day's biggest stories with the image front and center. | ||
| </h1> | ||
| <p className="max-w-[55ch] text-sm text-muted-foreground sm:text-base"> | ||
| Follow the same event across outlets, open the story page, and compare the underlying reporting without losing visual context. | ||
| </p> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🌐 Web query:
Are there design specs in the linked v0 chat (https://v0.app/chat/fFsRU4i2GT5) that prescribe this exact H1 wording?
💡 Result:
No. I could not find any publicly indexable text from the specific linked v0 chat (https://v0.app/chat/fFsRU4i2GT5) that prescribes “exact H1 wording”, and the only v0-related sources I could retrieve were general v0 documentation/pages rather than the contents of that specific chat. The public web results for that chat ID did not surface any H1 text or design-spec instructions to verify against your requested wording. If you paste the relevant section(s) of the chat (or the design-spec output it generated) here, I can check whether an H1 with that exact wording is explicitly specified.
Citations:
- 1: https://v0.app/faq
- 2: https://v0.app/chat
- 3: https://v0.dev/docs/v0-platform-api/chats/chats.init
- 4: https://v0.app/chat/chat-window-design-wq37O49upRy
- 5: https://v0.app/chat/new-chat-rJSbmilZvXm
Consider whether the H1 should function as a page title rather than marketing copy.
The H1 reads as marketing messaging ("See the day's biggest stories with the image front and center.") rather than a descriptive page title. For better SEO and clarity, consider restructuring to use a shorter, semantic title like "News Feed" in the H1, and move the marketing copy to the <p> below.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/feed.tsx` around lines 86 - 91, The H1 currently contains
marketing copy rather than a concise page title; change the <h1> in the feed
route (the element rendering the current headline in
apps/web/src/routes/feed.tsx) to a short semantic title like "News Feed" and
move the existing marketing sentence ("See the day's biggest stories with the
image front and center.") into the adjacent <p> (or update the <p> content to
include that marketing sentence), preserving existing className styling
(text-4xl font-bold... and max-w-[55ch] text-sm...) so visual layout stays the
same while improving semantics/SEO.
Owner
Author
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: flvvius <102308258+flvvius@users.noreply.github.com>
Contributor
Resolved in commit 134b651. The 5 conflicting files were merged as follows:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/routes/dashboard.tsx (1)
38-193: 🧹 Nitpick | 🔵 TrivialStandardize gradient utilities to Tailwind v4 canonical syntax.
The code inconsistently uses
bg-gradient-to-b(lines 38, 53, 96) andbg-linear-to-b/bg-linear-to-br(lines 188, 193). In Tailwind v4,bg-linear-to-*is the canonical form;bg-gradient-to-*is no longer the standard syntax. Update the three instances tobg-linear-to-bfor consistency with the rest of the component and to conform to Tailwind v4 conventions.♻️ Suggested change
- <div className="bg-gradient-to-b from-background via-background to-muted/35 min-h-[calc(100vh-4rem)]"> + <div className="bg-linear-to-b from-background via-background to-muted/35 min-h-[calc(100vh-4rem)]">(apply the same swap on lines 53 and 96)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/dashboard.tsx` around lines 38 - 193, Three JSX root divs still use the old Tailwind class "bg-gradient-to-b"; update those occurrences to the v4 canonical "bg-linear-to-b" to match the other gradients (e.g., the top-level unauthenticated/login container, the AuthLoading container, and the loading container returned when currentUser/isAdmin are undefined). Search for the literal class "bg-gradient-to-b" in this file and replace each with "bg-linear-to-b" so gradient classnames are consistent with the existing "bg-linear-to-br" usage.
♻️ Duplicate comments (1)
apps/web/src/components/feed/event-card.tsx (1)
86-96:⚠️ Potential issue | 🟠 Major
pointer-events-nonedoesn't fix the underlying invalid HTML / a11y issue.Adding
pointer-events-noneonly suppresses mouse/touch interaction. The chips are still rendered as<button>elements nested inside the wrapping<Link>(<a>), which:
- Violates the HTML spec (no interactive content inside
<a>).- Leaves the buttons in the keyboard tab order, creating extra focus stops with no action.
- Still gets announced as "button" by screen readers, which is misleading since they are purely decorative labels.
Replace the
Buttonwith a non-interactive element such as a<span>(or yourBadgecomponent) so the chips are presentational only.🩹 Suggested fix
- {(topics.length > 0 ? topics : ["General"]).slice(0, isFeature ? 3 : 2).map((topic) => ( - <Button - key={topic} - type="button" - variant="outline" - size="sm" - className="h-7 rounded-full border-border/80 bg-background/70 px-3 text-xs pointer-events-none" - > - {topic} - </Button> - ))} + {(topics.length > 0 ? topics : ["General"]) + .slice(0, isFeature ? 3 : 2) + .map((topic) => ( + <span + key={topic} + className="inline-flex h-7 items-center rounded-full border border-border/80 bg-background/70 px-3 text-xs" + > + {topic} + </span> + ))}As per coding guidelines: "Verify accessibility (a11y) standards" for components under
**/components/**.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/feed/event-card.tsx` around lines 86 - 96, The topic chips currently render an interactive <Button> inside the wrapping Link (the topics.map in event-card.tsx using the Button component), which is invalid and inaccessible; replace that Button with a non-interactive element (for example a <span> or the existing Badge component) when mapping topics (respect the isFeature slice logic), remove button-only props (type/variant/size) and the pointer-events-none class, preserve the visual classes (h-7 rounded-full border... px-3 text-xs) on the non-interactive element, and ensure it is not focusable (no tabIndex) and not announced as a button (no role="button"; optionally add aria-hidden or role="text" if the chip is purely decorative).
🤖 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/routes/dashboard.tsx`:
- Around line 341-348: The "Debug info - only in development" comment is
misleading because the JSX block rendering privateData?.message (from
api.privateData.get) has no environment check and will render in production if a
message exists; either remove the comment or explicitly gate the render on the
dev environment (e.g., wrap the conditional with import.meta.env.DEV or
process.env.NODE_ENV === 'development') so the debug message is only shown
during development, and keep the existing privateData?.message guard as-is.
- Around line 459-463: The status text rendered from configMessage is currently
a plain <p> so screen readers won't be notified of updates; update the JSX that
renders configMessage (the conditional block using configMessage) to keep the
element mounted but only conditionally render the text, and add accessibility
attributes like role="status" and aria-live="polite" (or aria-live="assertive"
for error messages if you prefer to interrupt) — e.g., render an always-present
<p role="status" aria-live="polite" aria-atomic="true">{configMessage &&
configMessage}</p> or split into branches to set assertive for errors and polite
for success, ensuring the element remains in the DOM so subsequent updates are
announced.
In `@apps/web/src/routes/event`.$slug.tsx:
- Line 86: Replace the HTML entities used in JSX with literal characters: change
occurrences of "The event you're looking for doesn't exist." to use
straight apostrophes and replace any "←" occurrences with the left-arrow
character (←) within the JSX rendered by the Event route component in
apps/web/src/routes/event.$slug.tsx (look for the JSX text nodes that contain
these strings). Ensure the text nodes or JSX children in the component use plain
characters so React renders them directly.
In `@apps/web/src/routes/feed.tsx`:
- Line 87: Replace the HTML entity ' with a literal apostrophe in the JSX
string "See the day's biggest stories with the image front and center."
(inside the feed component JSX/returned markup) so it reads "See the day's
biggest stories with the image front and center."; update the JSX text node
only—no other logic changes needed.
---
Outside diff comments:
In `@apps/web/src/routes/dashboard.tsx`:
- Around line 38-193: Three JSX root divs still use the old Tailwind class
"bg-gradient-to-b"; update those occurrences to the v4 canonical
"bg-linear-to-b" to match the other gradients (e.g., the top-level
unauthenticated/login container, the AuthLoading container, and the loading
container returned when currentUser/isAdmin are undefined). Search for the
literal class "bg-gradient-to-b" in this file and replace each with
"bg-linear-to-b" so gradient classnames are consistent with the existing
"bg-linear-to-br" usage.
---
Duplicate comments:
In `@apps/web/src/components/feed/event-card.tsx`:
- Around line 86-96: The topic chips currently render an interactive <Button>
inside the wrapping Link (the topics.map in event-card.tsx using the Button
component), which is invalid and inaccessible; replace that Button with a
non-interactive element (for example a <span> or the existing Badge component)
when mapping topics (respect the isFeature slice logic), remove button-only
props (type/variant/size) and the pointer-events-none class, preserve the visual
classes (h-7 rounded-full border... px-3 text-xs) on the non-interactive
element, and ensure it is not focusable (no tabIndex) and not announced as a
button (no role="button"; optionally add aria-hidden or role="text" if the chip
is purely decorative).
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 37f9f05e-acde-47cd-8688-99594b943acf
📒 Files selected for processing (4)
apps/web/src/components/feed/event-card.tsxapps/web/src/routes/dashboard.tsxapps/web/src/routes/event.$slug.tsxapps/web/src/routes/feed.tsx
Comment on lines
+341
to
+348
| {/* Debug info - only in development */} | ||
| {privateData?.message && ( | ||
| <div className="rounded-[1rem] bg-muted/50 border border-border/70 p-4"> | ||
| <p className="text-xs text-muted-foreground font-mono"> | ||
| Debug: {privateData.message} | ||
| </p> | ||
| </div> | ||
| )} |
Contributor
There was a problem hiding this comment.
Misleading "only in development" comment — block renders in production too.
The comment claims this is dev-only, but the render is gated solely on privateData?.message truthiness, with no import.meta.env.DEV / process.env.NODE_ENV check. If api.privateData.get ever returns a message in production, it will be shown to authenticated users and could leak server-side debug info. Either gate it on the environment or drop the misleading comment.
🛡️ Proposed fix
- {/* Debug info - only in development */}
- {privateData?.message && (
+ {/* Debug info - only in development */}
+ {import.meta.env.DEV && privateData?.message && (
<div className="rounded-[1rem] bg-muted/50 border border-border/70 p-4">
<p className="text-xs text-muted-foreground font-mono">
Debug: {privateData.message}
</p>
</div>
)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/dashboard.tsx` around lines 341 - 348, The "Debug info -
only in development" comment is misleading because the JSX block rendering
privateData?.message (from api.privateData.get) has no environment check and
will render in production if a message exists; either remove the comment or
explicitly gate the render on the dev environment (e.g., wrap the conditional
with import.meta.env.DEV or process.env.NODE_ENV === 'development') so the debug
message is only shown during development, and keep the existing
privateData?.message guard as-is.
Comment on lines
+459
to
+463
| {configMessage && ( | ||
| <p className="text-sm text-muted-foreground"> | ||
| {configMessage} | ||
| </p> | ||
| )} |
Contributor
There was a problem hiding this comment.
configMessage status text needs ARIA live region.
configMessage conveys validation failures (e.g. "Min score must be a number between 1 and 20.") and async save outcomes ("Topic inference settings saved." / "Could not save settings…"). It's a status message that mutates after the form submits but is rendered as a plain <p>, so screen-reader users won't be notified when validation fails or when saving completes. Add role="status" and aria-live="polite" (use aria-live="assertive" if you want errors to interrupt). For symmetry, consider aria-live="assertive" only for the error branches, but polite on the single element is acceptable.
🩹 Suggested change
- {configMessage && (
- <p className="text-sm text-muted-foreground">
- {configMessage}
- </p>
- )}
+ <p
+ role="status"
+ aria-live="polite"
+ className="text-sm text-muted-foreground"
+ >
+ {configMessage}
+ </p>Note: keep the element mounted so the live region can announce subsequent updates; conditionally render the text only.
As per coding guidelines: "Forms must have aria-label on inputs and ARIA live regions (aria-live, role) on status messages".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/dashboard.tsx` around lines 459 - 463, The status text
rendered from configMessage is currently a plain <p> so screen readers won't be
notified of updates; update the JSX that renders configMessage (the conditional
block using configMessage) to keep the element mounted but only conditionally
render the text, and add accessibility attributes like role="status" and
aria-live="polite" (or aria-live="assertive" for error messages if you prefer to
interrupt) — e.g., render an always-present <p role="status" aria-live="polite"
aria-atomic="true">{configMessage && configMessage}</p> or split into branches
to set assertive for errors and polite for success, ensuring the element remains
in the DOM so subsequent updates are announced.
| <h1 className="text-2xl font-semibold mb-2">Event not found</h1> | ||
| <p className="text-muted-foreground mb-4"> | ||
| The event you're looking for doesn't exist. | ||
| The event you're looking for doesn't exist. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unnecessary HTML entity encoding in JSX.
The HTML entities ' (apostrophe) and ← (left arrow) are unnecessary in React/JSX. Modern JSX handles these characters natively, and using entities reduces code readability without providing any benefit.
♻️ Simplify to use literal characters
- The event you're looking for doesn't exist.
+ The event you're looking for doesn't exist.- ← Back to feed
+ ← Back to feedAlso applies to: 116-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/event`.$slug.tsx at line 86, Replace the HTML entities
used in JSX with literal characters: change occurrences of "The event
you're looking for doesn't exist." to use straight apostrophes and
replace any "←" occurrences with the left-arrow character (←) within the
JSX rendered by the Event route component in apps/web/src/routes/event.$slug.tsx
(look for the JSX text nodes that contain these strings). Ensure the text nodes
or JSX children in the component use plain characters so React renders them
directly.
| <h1 className="text-4xl font-bold tracking-tight text-foreground sm:text-5xl"> | ||
| See the day’s biggest stories with the image front and center. | ||
| <h1 className="text-4xl font-bold tracking-tight text-foreground sm:text-5xl text-balance"> | ||
| See the day's biggest stories with the image front and center. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unnecessary HTML entity encoding in JSX.
In React/JSX, apostrophes can be written directly in strings without encoding them as '. The entity encoding reduces readability without providing any benefit, as JSX natively handles these characters.
♻️ Simplify to use a literal apostrophe
- See the day's biggest stories with the image front and center.
+ See the day's biggest stories with the image front and center.📝 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.
Suggested change
| See the day's biggest stories with the image front and center. | |
| See the day's biggest stories with the image front and center. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/feed.tsx` at line 87, Replace the HTML entity ' with
a literal apostrophe in the JSX string "See the day's biggest stories with
the image front and center." (inside the feed component JSX/returned markup) so
it reads "See the day's biggest stories with the image front and center.";
update the JSX text node only—no other logic changes needed.
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.
v0 Session
Summary by CodeRabbit
Release Notes
New Features
Improvements