Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/web/src/routes/_view/changelog/$slug.tsx
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Overflow clipping risk: no negative margin trick means scrollable content may be clipped by parent padding

The parent container at apps/web/src/routes/_view/changelog/$slug.tsx:186 applies px-6 padding. For DownloadLinksHeroMobile (line 261), this is compensated with -mx-6 px-6 so the scroll area extends edge-to-edge. However, the RelatedReleases component's flex container at line 327 sits inside that same px-6 parent without any negative margin compensation. This means the horizontal scroll area is constrained within the padded area — the first and last items won't be able to scroll flush to the screen edge, and depending on the number of items, the scroll area might be unnecessarily narrow. This isn't a bug per se (the scroll still works), but it's a different UX from the download links section and may look awkward on very narrow screens.

(Refers to lines 186-191)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ function RelatedReleases({
<p className="text-neutral-600">Explore more versions of Char</p>
</div>

<div className="grid gap-4 grid-cols-5">
<div className="flex overflow-x-auto gap-4 sm:grid sm:grid-cols-5 sm:overflow-visible">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Inconsistency with DownloadLinksHeroMobile scroll pattern — missing scrollbar-hide

The DownloadLinksHeroMobile component at apps/web/src/routes/_view/changelog/$slug.tsx:261 uses overflow-x-auto scrollbar-hide along with negative margin trick (-mx-6 px-6) to create an edge-to-edge scrollable area with hidden scrollbars. The new RelatedReleases mobile layout at line 327 uses overflow-x-auto but does NOT use scrollbar-hide or the negative margin pattern. The PR description even notes this as an open question. This is a stylistic inconsistency — the download links section hides its scrollbar while the Other Releases section would show one. Whether this is intentional or not depends on design preference, but it's worth the reviewer confirming the desired behavior.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

{relatedChangelogs.map((release) => {
const version = semver.parse(release.version);
const isPrerelease = version && version.prerelease.length > 0;
Expand All @@ -346,7 +346,7 @@ function RelatedReleases({
>
<article
className={cn([
"flex flex-col items-center gap-2 p-4 rounded-lg transition-all duration-300",
"flex flex-col items-center gap-2 p-4 rounded-lg transition-all duration-300 shrink-0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 shrink-0 applied to wrong element — flex items can still shrink on mobile

The shrink-0 class is added to the <article> element at line 349, but the direct children of the flex container (div.flex.overflow-x-auto at line 327) are the <Link> elements at line 338. In CSS flexbox, flex-shrink: 0 only prevents shrinking when applied to the direct children of the flex container. Since <Link> doesn't have shrink-0, the links will still shrink to fit the viewport on mobile, defeating the purpose of the horizontal scroll fix.

Root Cause

The flex container is:

<div class="flex overflow-x-auto gap-4 sm:grid sm:grid-cols-5 sm:overflow-visible">

Its direct children are <Link> elements:

<Link
  key={release.slug}
  className={cn(["group block", isCurrent && "pointer-events-none"])}
>

But shrink-0 is placed on the nested <article> inside the <Link>:

<article className={cn(["... shrink-0", ...])}>

Since <Link> renders as an <a> tag (a block element per the group block class), it is the flex item that will shrink. The shrink-0 on the inner <article> has no effect on the flex layout. The <Link> elements need shrink-0 to prevent compression in the mobile flex layout.

Impact: On mobile viewports, the release cards will still be compressed/crowded instead of scrolling horizontally, which is the exact issue this PR is trying to fix.

Suggested change
"flex flex-col items-center gap-2 p-4 rounded-lg transition-all duration-300 shrink-0",
"flex flex-col items-center gap-2 p-4 rounded-lg transition-all duration-300",
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

isCurrent ? "bg-stone-100" : "hover:bg-stone-50",
])}
>
Expand Down