feat(share): render tracks + runtime on /p/$token (phase 1.j.c)#22
Conversation
Closes the Phase 1.j loop on the web side. The DTO field was already there since 1.g.2, but the route previously rendered an ordered list with title + artist only — duration_ms was carried but ignored, and the page header showed neither a track count nor a runtime. Wire-side state today: - Server populates `tracks[]` from `playlist_track.snapshot_*` since PR #32 (1.j.a). - Desktop emits the snapshots since PR #203 (1.j.b). - Pre-1.j.b clients keep their playlists invisible to the public preview (server filters `snapshot_title IS NOT NULL`); the fallback "Track list preview is not available yet" message kicks in for those, unchanged. New rendering: - A "X tracks · 32 min" sub-header (1 track / N tracks singularisation; "Xh YY" for > 60 min, two-digit minute pad). - Tracks list switches from `<ol>` to a flex-row layout with position number + title/artist + right-aligned duration column, divided by hairlines. `tabular-nums` so `5:21` and `12:03` line up the same pixel width on a font that defaults to proportional digits. `truncate` on the title cell prevents long titles from pushing the duration column off-screen. - The duration column is dropped per-row when the value is non- positive (pre-1.j.b snapshot without a duration), so we never print `0:00`. - The `<meta name="description">` head tag uses the same "X tracks · 32 min" helper as the rendered header so the Discord / iMessage / Slack embed reads the same as the open tab. New module `src/lib/share-format.ts` holds `formatDuration` + `formatTrackCountAndRuntime` so a unit test can exercise the formatters without piggybacking on the TanStack file route. **10 unit tests pass** covering sub-hour mm:ss, hour h:mm:ss, ms rounding, non-positive / NaN / Infinity guard, singular/plural, empty list, all-zero-duration fallback, > 60 min "Xh YY", single-digit remainder padding, negative-duration clamp. Total suite: 76/76 tests pass. Server-fn DTO doc updated to drop the "currently always empty" qualifier on `tracks`. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
Warning Review limit reached
More reviews will be available in 14 minutes and 53 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAjout de deux fonctions de formatage pour enrichir l'aperçu public des playlists partagées : ChangesPublic Playlist Share Preview
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/routes/p`.$token.tsx:
- Around line 170-177: Update the misleading comment to accurately describe that
the code uses an ordered list (<ol>) styled with CSS (flex/columns and
tabular-nums) to simulate table-like column alignment rather than a semantic
<table>; reference the visible "value" column that preserves semantic row
positions for screen readers and explain that tabular-nums keeps duration
strings (e.g., "5:21", "12:03") fixed-width to avoid jitter when right-aligning
the duration column.
- Line 159: Replace Tailwind's old arbitrary value syntax using var() in
className attributes with the v4-compatible parenthesized CSS variable form:
find occurrences of text-[var(--sea-ink)] and text-[var(--sea-ink-soft)] in the
JSX (the p.$token.tsx render/JSX where those className strings are used) and
change them to text-(--sea-ink) and text-(--sea-ink-soft) respectively so
Tailwind v4 will emit var() automatically; update all instances in that file and
run the build/test to confirm no remaining usages of text-[var(...)] remain.
🪄 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 Plus
Run ID: b5ffca1d-b5e1-4a31-afcd-5d7a1f6bd80f
📒 Files selected for processing (4)
src/lib/share-format.test.tssrc/lib/share-format.tssrc/routes/p.$token.tsxsrc/server-fns/share.ts
CodeRabbit + CI feedback in one round. 1. **Comment was inverted.** Wrote "Table rather than `<ol>`" but the code uses `<ol>` styled with flex + `tabular-nums` to mimic table column alignment. Rewrite the comment to describe what's actually there: ordered list (kept for the screen-reader semantic) with a fixed-width position column and a tabular-nums duration cell, plus the rationale for the explicit `<span>` position number over the browser's automatic `<ol>` marker. 2. **Lockfile drift.** CI's `bun install --frozen-lockfile` failed after a dependency bump merged on main while this branch was in review. `bun install` reconciled (no version changes to the listed packages, just lockfile state) — commit the regenerated `bun.lock` so the CI gate flips green. Skipping the Tailwind v4 paren-syntax suggestion (`text-[var(--x)]` → `text-(--x)`): the whole repo uses the bracket form (Header.tsx, Footer.tsx, Player.tsx, the index landing page, …), so migrating just this file would create an inconsistency. The migration is worth doing across the board, but belongs in the Sprint 3 design-system pass that already touches these tokens — not in a feature PR. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
@coderabbitai trois actions : 1. Lockfile (CI fix) — 2. Comment fixed (finding #1) — j'avais écrit "Table rather than 3. Tailwind v4 paren syntax (finding #2) → skipped avec rationale : la suggestion ( Push: f90d6d7. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# List open review comments from coderabbitai on PR `#22` to find the ones to resolve
gh api repos/InstaZDLL/waveflow-web/pulls/22/comments \
--jq '.[] | select(.user.login == "coderabbitai") | {id: .id, body: .body[:120]}'Length of output: 478 🏁 Script executed: #!/bin/bash
# Check review threads via GraphQL to find unresolved ones from coderabbitai
gh api graphql -f query='
{
repository(owner: "InstaZDLL", name: "waveflow-web") {
pullRequest(number: 22) {
reviewThreads(first: 50) {
nodes {
id
isResolved
comments(first: 1) {
nodes {
author { login }
body
}
}
}
}
}
}
}' | jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false) | {id: .id, author: .comments.nodes[0].author.login, body: .comments.nodes[0].body[:100]}'Length of output: 855 🏁 Script executed: #!/bin/bash
# Resolve the unresolved coderabbitai thread
gh api graphql -f query='
mutation {
resolveReviewThread(input: {threadId: "PRRT_kwDOSsfiLs6HmpxD"}) {
thread {
id
isResolved
}
}
}'Length of output: 350
✏️ Learnings added
🧠 Learnings used |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/p.$token.tsx (1)
1-262:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFormatter le fichier avec Prettier avant merge.
Le CI signale un problème de formatting Prettier sur ce fichier. Lance
prettier --write src/routes/p.\$token.tsxpour corriger.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/p`.$token.tsx around lines 1 - 262, Prettier formatting failed for this route file; run Prettier to fix and commit the result. Run prettier --write focusing on the file that declares Route via createFileRoute('/p/$token') (the component and helpers: PublicPlaylistView, PlaylistPanel, NotFoundPanel, ErrorPanel, colorTileClass) so formatting matches the repo config, then add/commit the formatted changes and push.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/routes/p`.$token.tsx:
- Around line 1-262: Prettier formatting failed for this route file; run
Prettier to fix and commit the result. Run prettier --write focusing on the file
that declares Route via createFileRoute('/p/$token') (the component and helpers:
PublicPlaylistView, PlaylistPanel, NotFoundPanel, ErrorPanel, colorTileClass) so
formatting matches the repo config, then add/commit the formatted changes and
push.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1f35c02c-58c8-444d-af01-3c47e56b16ac
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!bun.lock,!*.lock
📒 Files selected for processing (1)
src/routes/p.$token.tsx
Two follow-ups in one round. 1. **Prettier formatting** (CR finding). Ran `prettier --write` on the route + the lib module + its test. Pure whitespace. 2. **Canonical share URL** (closes #21). The route already vended `og:type`, `og:title`, `og:description`, `og:site_name`, but no `og:url` — some scrapers (Slack, Discord, Meta) then fall back to the referrer URL instead of the canonical share path, which degrades the preview quality. Added: `og:url`, `twitter:url`, and the HTML-standard `<link rel="canonical">` (Google's index + Facebook's debugger key off this), all built from `${getCanonicalOrigin()}/p/${params.token}`. New helper `getCanonicalOrigin()` in `src/lib/share-format.ts` reads `BETTER_AUTH_URL` (already the source of truth for the web origin — set both in dev .env and in prod / preview deploys) with a `https://waveflow.app` fallback. Trailing slashes are stripped so the final URL doesn't double-slash. Empty env is treated as unset (some shells export `Foo=""` which surfaces as `Ok("")` and a bare slash would be useless to any scraper). `head()` runs SSR-side at the moment a crawler hits the page, so `process.env` is available; the value is read inline rather than captured in a const because `head()` re-runs per request and a closure-captured value would freeze on the first SSR. `head` callback now also reads `params` from the per-route context — TanStack passes both alongside `loaderData`. Tests: 4 new on `getCanonicalOrigin` (env set, trailing-slash strip, unset fallback, empty-string fallback). Suite: 80/80. Skipping the Tailwind v4 paren-syntax suggestion from round 1 with the same rationale as before — repo-wide migration belongs in the Sprint 3 design-system pass, not in this feature PR. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
@coderabbitai deux corrections + close issue #21 : 1. Prettier formatting — 2. Canonical share URL (closes #21) — la route vendait déjà Ajouté :
Tests : 4 nouveaux sur Tailwind v4 paren-syntax du round 1 : toujours skipped avec même rationale (repo-wide migration belong Sprint 3 DS pass). Push : 5253d59. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check unresolved review threads from coderabbitai on PR `#22`
gh api graphql -f query='
{
repository(owner: "InstaZDLL", name: "waveflow-web") {
pullRequest(number: 22) {
reviewThreads(first: 50) {
nodes {
id
isResolved
comments(first: 1) {
nodes {
author { login }
body
}
}
}
}
}
}
}' | jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false) | {id: .id, author: .comments.nodes[0].author.login, body: .comments.nodes[0].body[:120]}'
# Check state of issue `#21`
gh issue view 21 --repo InstaZDLL/waveflow-web --json number,title,state,closedAtLength of output: 890 🏁 Script executed: #!/bin/bash
# Close issue `#21` with a reference to PR `#22`
gh issue close 21 --repo InstaZDLL/waveflow-web \
--comment "Résolu par le commit 5253d59 dans la PR `#22` : \`og:url\`, \`twitter:url\` et \`<link rel=\"canonical\">\` ajoutés via le helper \`getCanonicalOrigin()\` (lit \`BETTER_AUTH_URL\`, fallback \`https://waveflow.app\`, trailing-slash strippé)."Length of output: 717
🐇 ✨ 🧠 Learnings used |
1 participant
Summary
Closes the Phase 1.j loop on the web side. The DTO field was already there since 1.g.2 (the route just rendered an empty fallback), but the route previously kept rendering an ordered list with title + artist only —
duration_mswas carried through the wire but ignored, and the page header showed neither a track count nor a runtime.End-to-end state today
Pre-1.j.b clients keep their playlists invisible to the public preview (server filters `snapshot_title IS NOT NULL`); the existing "Track list preview is not available yet" fallback stays in place for those.
New rendering
` to a flex-row layout with position number + title/artist + right-aligned duration column, divided by hairlines.- `tabular-nums` so `5:21` and `12:03` line up the same pixel width on a font with proportional digits by default.
- `truncate` on the title cell prevents long titles from pushing the duration column off-screen.
- Per-row duration column dropped when the value is non-positive (defensive against an edge-case where one snapshot doesn't carry the duration) — we never print `0:00`.
- `<meta name="description">` head tag uses the same helper as the rendered header so the Discord / iMessage / Slack embed reads the same as the open tab.
New module
`src/lib/share-format.ts` holds `formatDuration` + `formatTrackCountAndRuntime` so a unit test can exercise the formatters without piggybacking on the TanStack file route (which interprets every export).
Test plan
Follow-up
This concludes Phase 1.j. Sprint 2 next tasks per the post-1.g sprint plan: OAuth providers (Google + Apple via Better Auth), then RFC-004 Community-DB draft (design only).
Summary by CodeRabbit
Nouvelles Fonctionnalités
Tests