Sticky Header Or No Sticky Header ID-640#1166
Conversation
📝 WalkthroughWalkthroughThis PR applies a systematic Tailwind v4 cleanup across the frontend codebase, replacing arbitrary value classes (e.g., ChangesTailwind v4 Arbitrary-to-Canonical Class Cleanup
🚥 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
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@fix_next.md`:
- Line 137: The note about ID-640 is confusing because it claims
`class-detail/index.tsx` `z-[100]` and `Schedule.tsx` sizing were "already
fixed" in branch cpride/class-detail-a11y, yet the current diff shows those
exact changes (`class-detail/index.tsx` z-100 and `Schedule.tsx` h-150 and
max-w-350) being applied in this PR; update the note to either remove the
statement or reword it to explicitly state that these changes to
`class-detail/index.tsx` (z-100) and `Schedule.tsx` (h-150, max-w-350) are
included and being applied in this PR (ID-640) as part of the sticky header work
rather than being pre-existing in the other branch.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 59437420-8fb7-4aeb-b47f-68e11e122e52
📒 Files selected for processing (7)
fix_next.mdfrontend/src/layouts/AuthenticatedLayout.tsxfrontend/src/pages/Schedule.tsxfrontend/src/pages/class-detail/RosterTab.tsxfrontend/src/pages/class-detail/SessionRow.tsxfrontend/src/pages/class-detail/index.tsxfrontend/src/styles/globals.css
| ## Notes | ||
|
|
||
| - `components/ui/` files (e.g. `command.tsx`, `drawer.tsx`, `navigation-menu.tsx`) are project-owned shadcn components — fix them the same as application code | ||
| - `class-detail/index.tsx` `z-[100]` and `Schedule.tsx` `h-[600px]` / `max-w-[1400px]` were already fixed in branch `cpride/class-detail-a11y` (ID-640) |
There was a problem hiding this comment.
Clarify the note about ID-640 changes.
This note states that class-detail/index.tsx z-[100] and Schedule.tsx sizing were "already fixed" in the cpride/class-detail-a11y branch, but those exact changes appear in the current diff (class-detail/index.tsx line 173: z-100, Schedule.tsx lines 350, 367: h-150, max-w-350). Since this PR is also ID-640, the note creates confusion about whether these changes are in scope.
Consider either removing this note or rewording it to clarify that these specific files are being updated in this PR as part of the sticky header work.
🤖 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 `@fix_next.md` at line 137, The note about ID-640 is confusing because it
claims `class-detail/index.tsx` `z-[100]` and `Schedule.tsx` sizing were
"already fixed" in branch cpride/class-detail-a11y, yet the current diff shows
those exact changes (`class-detail/index.tsx` z-100 and `Schedule.tsx` h-150 and
max-w-350) being applied in this PR; update the note to either remove the
statement or reword it to explicitly state that these changes to
`class-detail/index.tsx` (z-100) and `Schedule.tsx` (h-150, max-w-350) are
included and being applied in this PR (ID-640) as part of the sticky header work
rather than being pre-existing in the other branch.
There was a problem hiding this comment.
This is because the a11y branch has not been merged. So same changes are being applied multiple times thus the need for this to be fixed app wide. I am creating a PR to clean it up in one sweep. That PR will follow this PR.
Removed overflow-y-auto overflow-x-hidden from the authenticated layout's content area (which was silently breaking position: sticky), then added sticky headers to the Class Detail and Schedule pages.
pages/class-detail/index.tsx) — white header bar (breadcrumbs + class title + action buttons) is nowsticky top-0 z-30, pinning at 64px (below TopNav) on scrollpages/Schedule.tsx) — page header row (title + filter + New Event button) is nowsticky top-0 z-30with a full-width background strip (-mx-8 px-8) that prevents edge bleed on scrolllayouts/AuthenticatedLayout.tsx) — removedoverflow-y-auto overflow-x-hiddenfromcontentClass; these properties were creating an intermediate scroll context that silently interceptedposition: stickybefore it could reach the real scroll container. Without this fix, both sticky headers appeared to work but scrolled away with the page content.pages/Schedule.tsx) — removedoverflow-x-hiddenfrom the root div; per CSS spec,overflow-x: hiddenforcesoverflow-y: auto, adding a second intercepting scroll context that broke sticky on the Schedule page specifically.suggestCanonicalClasseswarnings resolved for files edited in this PR (see details below)While making the sticky header changes, ESLint's
suggestCanonicalClassesrule flagged arbitrary-value Tailwind classes (e.g.w-[140px],z-[100]) in the files I was already editing. These warnings are a byproduct of the project's migration to Tailwind v4 — v4 introduced first-class canonical equivalents for many previously arbitrary values, and the lint rule now requires them. Because the rule applies to any file touched in a PR, each file I edited had to have its violations resolved before the lint check would pass. This has been the case on at least the last 4 PRs that I have submitted, and the workflow has been that I resolved the warnings in those files only — the fixes are purely syntactic with no visual impact. The fact remains that the rest of the codebase still has similar warnings site-wide, so a dedicated follow-up PR will sweep those separately.Canonical class fixes (files touched by this PR)
class-detail/index.tsxz-[100]z-100class-detail/RosterTab.tsxw-[140px],z-[100]w-35,z-100class-detail/SessionRow.tsxflex-shrink-0(×6)shrink-0(×6)Schedule.tsxh-[600px],max-w-[1400px]h-150,max-w-350styles/globals.cssw-[220px],flex-shrink-0(×2)w-55,shrink-0(×2)Files changed
```
frontend/src/layouts/AuthenticatedLayout.tsx bug fix: remove overflow from contentClass
frontend/src/pages/Schedule.tsx sticky header + overflow fix + canonical classes
frontend/src/pages/class-detail/index.tsx sticky header + canonical class
frontend/src/pages/class-detail/RosterTab.tsx canonical classes only
frontend/src/pages/class-detail/SessionRow.tsx canonical classes only
frontend/src/styles/globals.css canonical classes only
```