Conversation
|
@Alqamaq is attempting to deploy a commit to the Ketan's Personal Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Changelog Page app/changelog/page.tsx |
New Next.js app page exporting metadata and default ChangelogPage. Sorts CHANGELOG_ENTRIES by timeStamp, formats markdown-style links, and renders a responsive two-column timeline with date/version badges, desktop & mobile PR links, and Header/FooterSmall wrappers. |
Changelog Data lib/changelog-data.ts |
New ChangelogEntry interface and exported CHANGELOG_ENTRIES array with two sample entries (date, timeStamp, version, title, summary, prLinks, changes). |
Footer Navigation components/footer-common.tsx |
Added "Changelog" (/changelog) to the Community section of FOOTER_NAVIGATION_ITEMS. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
🐇 I hopped through commits, a tidy trail,
Dates, versions, links in teal detail,
Badges gleam and PRs in view,
Footer points where changelogs flew,
A happy hop — the updates tell the tale!
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | The PR description covers the summary section with detailed information about implementation approach and data structure. However, the testing section only contains an image without written test instructions, and the checklist is missing entirely. | Complete the checklist section with checkboxes marked as done/not applicable. Add written test instructions or verification steps beyond just the screenshot image. | |
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title 'feat: add changelog page' clearly and concisely summarizes the primary change in the changeset, which is the addition of a new changelog page feature. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@app/changelog/page.tsx`:
- Around line 76-80: The anchor rendering for PR links (the <a key={i}
href={pr.link} target="_blank" ...> element) is missing rel="noopener
noreferrer"; update that anchor to include rel="noopener noreferrer" to mitigate
reverse-tabnabbing, and scan other uses of target="_blank" in this component
(e.g., any anchors that render external PR or changelog links) to apply the same
rel attribute consistently.
- Around line 29-35: The anchor elements using hardcoded color utilities (e.g.,
the <a key={match.index} href={match[2]} ... className="text-teal-600
dark:text-teal-400 ...">) must be changed to use existing design tokens instead
of literal teal classes; replace "text-teal-600 dark:text-teal-400" with the
tokenized class (for example "text-accent-foreground" or the appropriate
existing token used elsewhere) and keep the other utilities (hover:underline,
font-medium, transition-colors) intact; apply the same replacement for the other
anchor/className occurrences flagged (the similar className strings around the
later anchor usages).
- Around line 8-11: The metadata object for the changelog page is missing a
canonical URL; import getFullUrl from "@/lib/constants" (if not already) and add
an alternates.canonical property to the exported metadata (e.g., alternates: {
canonical: getFullUrl("/changelog") }) so the page follows the same SEO pattern
used by other pages; update the metadata export (export const metadata:
Metadata) to include this new alternates.canonical entry.
In `@lib/changelog-data.ts`:
- Around line 22-31: The changelog contains placeholder URLs in prsLink, images,
and the inline markdown URL inside the changes array (e.g., prsLink entries,
images array, and the 'Added new [filtering system](https://github.com/...)'
text); replace each placeholder "https://github.com/..." with the actual PR URLs
(use the full repo+PR path) and swap the placeholder image URL in images with
the real preview image URL; also update any other occurrences of these
placeholders elsewhere in the file (including other entries referenced by the
changes array) so all PR links, inline markdown links, and preview images are
authoritative before release.
🧹 Nitpick comments (1)
app/changelog/page.tsx (1)
105-110: Prefernext/imagefor changelog screenshots.
Lint warns about raw<img>; usingnext/imageimproves LCP and bandwidth. Ensure the remote domain is allow‑listed innext.configand set appropriate dimensions.🖼️ Example migration to
next/image-import { Metadata } from "next"; +import { Metadata } from "next"; +import Image from "next/image";- <img key={i} src={img} alt="Update" className="w-full h-auto" /> + <Image + key={i} + src={img} + alt="Update" + className="w-full h-auto" + width={1200} + height={600} + />
| <a | ||
| key={match.index} | ||
| href={match[2]} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="text-teal-600 dark:text-teal-400 hover:underline font-medium transition-colors" | ||
| > |
There was a problem hiding this comment.
Replace hardcoded teal colors with existing design tokens.
text-teal-600 / dark:text-teal-400 introduce hardcoded colors that can drift from the theme. Please use existing tokenized classes (e.g., text-accent-foreground) so dark/light modes stay consistent.
🎨 Example tokenized swap
- className="text-teal-600 dark:text-teal-400 hover:underline font-medium transition-colors"
+ className="text-accent-foreground hover:underline font-medium transition-colors"- className="flex items-center gap-1.5 text-xs text-teal-600 dark:text-teal-400 hover:underline transition-colors group"
+ className="flex items-center gap-1.5 text-xs text-accent-foreground hover:underline transition-colors group"- <GitHubIcon className="w-3 h-3 text-muted-foreground group-hover:text-teal-600" />
+ <GitHubIcon className="w-3 h-3 text-muted-foreground group-hover:text-accent-foreground" />- className="flex items-center gap-2 text-sm text-teal-600 dark:text-teal-400 font-medium"
+ className="flex items-center gap-2 text-sm text-accent-foreground font-medium"Also applies to: 80-83, 121-122
🤖 Prompt for AI Agents
In `@app/changelog/page.tsx` around lines 29 - 35, The anchor elements using
hardcoded color utilities (e.g., the <a key={match.index} href={match[2]} ...
className="text-teal-600 dark:text-teal-400 ...">) must be changed to use
existing design tokens instead of literal teal classes; replace "text-teal-600
dark:text-teal-400" with the tokenized class (for example
"text-accent-foreground" or the appropriate existing token used elsewhere) and
keep the other utilities (hover:underline, font-medium, transition-colors)
intact; apply the same replacement for the other anchor/className occurrences
flagged (the similar className strings around the later anchor usages).
✅ Addressed in commit 4e68fe2
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@Alqamaq have a look on the codeRabbit comments. |
ketankauntia
left a comment
ketankauntia
left a comment
There was a problem hiding this comment.
Also, have a writeup in the end as to how new changelogs can be added, how they get stored and other imp things, so that it is easier to navigate and understand for others as well.
good for future reference if we come to work on this feature again,
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/changelog/page.tsx`:
- Around line 67-68: Replace the unstable array index key used in the
sortedEntries.map render with a stable identifier: change the JSX key on the
article element (currently key={idx} in the sortedEntries.map callback) to use
the entry.timeStamp property (or another unique, stable id on the entry) so
React can reconcile items correctly when entries are inserted or reordered.
| <a | ||
| key={match.index} | ||
| href={match[2]} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="text-teal-600 dark:text-teal-400 hover:underline font-medium transition-colors" | ||
| > |
Alqamaq
force-pushed
the
feat/changelog
branch
from
09cb4c0 to
d36e0cc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/changelog/page.tsx`:
- Line 60: Replace the invalid Tailwind class mt-15 on the header element with a
valid design-system token or an arbitrary value; locate the header with
className "mb-16 mt-15 border-b border-border pb-8" and swap mt-15 for one of
the semantic tokens (e.g., mt-2xl for 48px) or an explicit utility (e.g.,
mt-[60px]) to match the intended spacing.
🧹 Nitpick comments (3)
app/changelog/page.tsx (3)
22-23: Trim obvious inline comments.The comment at Line 22 (and similar ones at Line 66, Line 72, Line 97, Line 113) restates what the JSX already makes clear; removing them keeps the file concise.
As per coding guidelines: Do NOT add comments that restate the obvious or explain what the code does if clear. ONLY add comments for non-obvious decisions, workarounds, edge cases, and architectural reasoning.
34-40: Avoid inline link colors; use tokenized classes.Inline
stylecolors override theme tokens and can drift from dark/light mode rules. Prefer existing text token classes for links, and apply the same change to the PR link anchors at Line 82-89 and Line 116-120. If this removes the last usage, drop thechartColorsimport.♻️ Suggested change
- <a + <a key={match.index} href={match[2]} target="_blank" rel="noopener noreferrer" - style={{ color: chartColors.accent.primary }} - className="hover:underline font-medium transition-colors" + className="text-accent-foreground hover:underline font-medium transition-colors" >- <a + <a key={i} href={pr.link} target="_blank" rel="noopener noreferrer" - style={{ color: chartColors.accent.primary }} className="flex items-center gap-1.5 text-xs text-accent-foreground hover:underline transition-colors group" >- <a + <a key={i} href={pr.link} - style={{ color: chartColors.accent.primary }} className="flex items-center gap-2 text-sm text-accent-foreground font-medium" >As per coding guidelines: Use ONLY existing color tokens and design variables, do NOT hardcode colors, do NOT introduce new colors without explicit instruction; Respect dark/light mode rules if present in the design system.
81-83: Prefer stable keys over array indexes for lists.Using
ican cause incorrect reconciliation if items are inserted or reordered. Use a stable identifier (e.g.,pr.link/pr.number, and a stablechangefield).♻️ Suggested change
- {entry.prLinks.map((pr, i) => ( + {entry.prLinks.map((pr) => ( <a - key={i} + key={pr.link} href={pr.link} target="_blank" rel="noopener noreferrer" className="flex items-center gap-1.5 text-xs text-accent-foreground hover:underline transition-colors group" >- {entry.changes.map((change, i) => ( - <li key={i} className="flex items-start gap-3"> + {entry.changes.map((change) => ( + <li key={change.text} className="flex items-start gap-3">Also applies to: 105-107
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/changelog/page.tsx`:
- Around line 34-41: The anchor elements are using an inline style prop
(style={{ color: chartColors.accent.primary }}) which hardcodes color and breaks
token/dark-mode rules; remove the style prop and replace it with the appropriate
existing design token class (e.g., add the tokenized text color class to
className such as text-accent-primary or the project’s equivalent token class)
and then remove the chartColors import if it becomes unused; apply the same
replacement for the other anchor occurrences referenced (the anchors at the
other ranges) so all links use tokenized classes instead of inline styles.
🧹 Nitpick comments (1)
app/changelog/page.tsx (1)
65-67: Remove the obvious timeline comment.The comment restates what the markup already makes clear.
♻️ Proposed fix
- {/* The Timeline Line */} <div className="absolute left-[11px] md:left-[155px] top-2 bottom-0 w-px bg-border hidden sm:block" />As per coding guidelines: Do NOT add comments that restate the obvious or explain what the code does if clear.
Alqamaq
force-pushed
the
feat/changelog
branch
from
8a5fb7d to
5f90226
Compare
|
Looks good in deployment. I will review the pr once and release the PR. |
2 participants
Summary
Zero database latency for the feed.
Updates are made through PRs, keeping the "code" and "history" together.
All updates are stored in array like objects in
lib/changelog-data.tsSchema:
timeStamp: A numerical value (YYYYMMDD) used for automatic sorting.version: The release version stringprLinks: An array of objects{link: string, number: string}to support multiple prs.changes: An object with type of changes (fix,feat,chore) and desc of changes.lib/changelog-data.tsCHANGELOG_ENTRIESarrayformatChangelogTextwill automatically style these as Teal LinksTesting
Summary by CodeRabbit