Unify app headers and brand metadata#4
Conversation
- Replace bespoke admin chrome with shared `AppHeader` - Split product name/product branding for chat shell - Add reusable header component to `@wikipefia/ui`
📝 WalkthroughWalkthroughAdds ChangesShared AppHeader/AppWordmark Rollout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 docstrings
🧪 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/library/app/file/[id]/page.tsx (1)
148-155:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign the new Download CTA with tokenized colors and uppercase label rules.
The updated button composition still includes a hardcoded color (
text-white) and non-uppercase label text.Suggested patch
-import { FONT } from "`@/lib/theme`"; +import { C, FONT } from "`@/lib/theme`"; @@ className={cn( buttonVariants({ size: "sm" }), - "border-accent bg-accent text-white", + "uppercase tracking-wider", )} - style={{ fontFamily: FONT.mono }} + style={{ + fontFamily: FONT.mono, + borderColor: C.accent, + backgroundColor: C.accent, + color: C.bgWhite, + }} > - ↓ Download + ↓ DOWNLOAD </a>As per coding guidelines, "Use CSS variables (C.bg, C.bgWhite, C.text, C.textMuted, C.accent, C.border, C.borderLight, C.headerBg, C.headerText) imported from
@/lib/themefor all styling; never hardcode colors except for subject palette colors" and "Apply uppercase styling to all headers, labels, navigation, badges, and breadcrumbs using text-transform or Tailwind uppercase utility".🤖 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 `@apps/library/app/file/`[id]/page.tsx around lines 148 - 155, The Download button in the download link element uses a hardcoded color class text-white instead of a theme CSS variable, and the label text is not uppercase. Replace the hardcoded text-white in the className with an appropriate CSS variable from `@/lib/theme` (such as C.text or C.headerText based on design requirements), and apply uppercase styling to the button label by adding the Tailwind uppercase utility class to the className or wrapping the text with a text-transform style, ensuring the button follows the tokenized color system and uppercase label conventions defined in the coding guidelines.Source: Coding guidelines
🧹 Nitpick comments (1)
apps/studio/app/page.tsx (1)
814-880: ⚡ Quick winPrefer
AppHeaderfor the IDE path too, instead of a bespoke<header>.Line [814] reintroduces custom header chrome in Studio. Since
AppHeaderalready supportsbrandandrightslots, this path will otherwise drift from shared behavior over time.Refactor direction
- <header className="flex h-12 shrink-0 items-center justify-between border-b border-line bg-invert px-5 text-invert-fg"> - ... - </header> + <AppHeader + brand={ + <div className="flex items-center gap-3"> + <AppWordmark product="Studio" /> + {/* existing picker button + syncing indicator */} + </div> + } + right={ + <div className="flex items-center gap-2"> + {/* existing status indicator */} + <ThemeToggle + size="icon-sm" + className="border-invert-fg/25 text-invert-fg hover:border-accent hover:text-accent" + /> + </div> + } + themeToggle={false} + />🤖 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 `@apps/studio/app/page.tsx` around lines 814 - 880, Replace the custom `<header>` element (the one with className containing "flex h-12 shrink-0") with the `AppHeader` component instead. Move the left section containing the AppWordmark and project picker button into the `brand` slot of `AppHeader`, and move the right section containing the AnimatePresence and ThemeToggle into the `right` slot of `AppHeader`. This ensures the IDE path uses the same shared AppHeader component as other parts of the application, preventing future divergence in header behavior.
🤖 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 `@apps/library/components/masthead.tsx`:
- Around line 22-27: The Button component in the Upload action has two issues:
it uses a hardcoded color className text-white instead of theme color tokens,
and the button label is mixed-case. Replace the hardcoded text-white in the
className with the appropriate theme color token from the imported theme
constants (similar to how border-accent and bg-accent are already used), and
change the button label from plus Upload to uppercase format (either as UPLOAD
or by applying the appropriate text-transform or Tailwind uppercase utility
class) to align with the repository's styling conventions for headers, labels,
and navigation elements.
In `@packages/chat/src/react/components/ThreadSidebar.tsx`:
- Around line 4-6: The import statement on line 4 of ThreadSidebar.tsx imports
motion from framer-motion, which does not follow the coding guidelines for
animation libraries in TSX files. Change the import source from framer-motion to
motion/react for the motion import to comply with the requirement to use
motion/react for all animations in TSX files.
In `@packages/ui/src/components/app-header.tsx`:
- Around line 95-103: The AppHeader component in the outer div and inner flex
container is using outdated header layout specifications. Replace the
positioning classes `relative z-20` with `sticky top-0 z-40` in the outer
className, and change the height class from `h-12` to `h-14` in the inner flex
div className. This aligns the header with the required app header contract of
sticky positioning at the top with proper z-index and a 56px height instead of
the current 48px.
---
Outside diff comments:
In `@apps/library/app/file/`[id]/page.tsx:
- Around line 148-155: The Download button in the download link element uses a
hardcoded color class text-white instead of a theme CSS variable, and the label
text is not uppercase. Replace the hardcoded text-white in the className with an
appropriate CSS variable from `@/lib/theme` (such as C.text or C.headerText based
on design requirements), and apply uppercase styling to the button label by
adding the Tailwind uppercase utility class to the className or wrapping the
text with a text-transform style, ensuring the button follows the tokenized
color system and uppercase label conventions defined in the coding guidelines.
---
Nitpick comments:
In `@apps/studio/app/page.tsx`:
- Around line 814-880: Replace the custom `<header>` element (the one with
className containing "flex h-12 shrink-0") with the `AppHeader` component
instead. Move the left section containing the AppWordmark and project picker
button into the `brand` slot of `AppHeader`, and move the right section
containing the AnimatePresence and ThemeToggle into the `right` slot of
`AppHeader`. This ensures the IDE path uses the same shared AppHeader component
as other parts of the application, preventing future divergence in header
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: de973d66-a12d-4156-b410-1b94175156a3
📒 Files selected for processing (14)
apps/admin/components/layout/admin-shell.tsxapps/chat/components/ChatProvider.tsxapps/chat/tsconfig.tsbuildinfoapps/library/app/file/[id]/page.tsxapps/library/components/masthead.tsxapps/portal/components/shared/page-shell.tsxapps/portal/lib/theme.tsapps/studio/app/page.tsxapps/studio/lib/theme.tspackages/chat/src/react/components/ThreadSidebar.tsxpackages/chat/src/react/components/ThreadView.tsxpackages/chat/src/react/transport-context.tsxpackages/ui/src/components/app-header.tsxpackages/ui/src/components/index.ts
| <Button | ||
| size="sm" | ||
| onClick={onUpload} | ||
| className="border-accent bg-accent text-white hover:opacity-80" | ||
| > | ||
| + Upload |
There was a problem hiding this comment.
Use theme color tokens and uppercase label for the Upload action.
This new button uses a hardcoded color (text-white) and a mixed-case label, which breaks the TSX styling conventions in this repo.
Suggested patch
import { AppHeader, AppWordmark, Button } from "`@wikipefia/ui`";
import Link from "next/link";
+import { C } from "`@/lib/theme`";
@@
<Button
size="sm"
onClick={onUpload}
- className="border-accent bg-accent text-white hover:opacity-80"
+ className="uppercase tracking-wider hover:opacity-80"
+ style={{
+ borderColor: C.accent,
+ backgroundColor: C.accent,
+ color: C.bgWhite,
+ fontFamily: "var(--font-mono)",
+ }}
>
- + Upload
+ + UPLOAD
</Button>As per coding guidelines, "Use CSS variables (C.bg, C.bgWhite, C.text, C.textMuted, C.accent, C.border, C.borderLight, C.headerBg, C.headerText) imported from @/lib/theme for all styling; never hardcode colors except for subject palette colors" and "Apply uppercase styling to all headers, labels, navigation, badges, and breadcrumbs using text-transform or Tailwind uppercase utility".
🤖 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 `@apps/library/components/masthead.tsx` around lines 22 - 27, The Button
component in the Upload action has two issues: it uses a hardcoded color
className text-white instead of theme color tokens, and the button label is
mixed-case. Replace the hardcoded text-white in the className with the
appropriate theme color token from the imported theme constants (similar to how
border-accent and bg-accent are already used), and change the button label from
plus Upload to uppercase format (either as UPLOAD or by applying the appropriate
text-transform or Tailwind uppercase utility class) to align with the
repository's styling conventions for headers, labels, and navigation elements.
Source: Coding guidelines
| import { motion } from "framer-motion"; | ||
| import { C } from "@wikipefia/mdx-renderer/theme"; | ||
| import { AppWordmark } from "@wikipefia/ui"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify remaining imports that still use framer-motion in React view files.
rg -n --iglob '**/*.{tsx,jsx}' 'from ["'\'']framer-motion["'\'']|require\(["'\'']framer-motion["'\'']\)'Repository: Wikipefia/wikipefia-ck
Length of output: 1767
Switch animation import from framer-motion to motion/react.
Line 4 uses framer-motion in a TSX file, but the standard is motion/react.
Proposed fix
-import { motion } from "framer-motion";
+import { motion } from "motion/react";Per coding guidelines, Use motion/react for all animations; no other animation libraries in .tsx files.
📝 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.
| import { motion } from "framer-motion"; | |
| import { C } from "@wikipefia/mdx-renderer/theme"; | |
| import { AppWordmark } from "@wikipefia/ui"; | |
| import { motion } from "motion/react"; | |
| import { C } from "`@wikipefia/mdx-renderer/theme`"; | |
| import { AppWordmark } from "`@wikipefia/ui`"; |
🤖 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 `@packages/chat/src/react/components/ThreadSidebar.tsx` around lines 4 - 6, The
import statement on line 4 of ThreadSidebar.tsx imports motion from
framer-motion, which does not follow the coding guidelines for animation
libraries in TSX files. Change the import source from framer-motion to
motion/react for the motion import to comply with the requirement to use
motion/react for all animations in TSX files.
Source: Coding guidelines
| "relative z-20 shrink-0 border-b border-line bg-invert text-invert-fg", | ||
| className, | ||
| )} | ||
| {...rest} | ||
| > | ||
| <div | ||
| className={cn( | ||
| "flex h-12 items-center justify-between px-5", | ||
| innerClassName, |
There was a problem hiding this comment.
Align shared header layout contract (h-14 + sticky positioning).
Line [95] and Line [102] currently render a 48px non-sticky header (h-12, relative z-20). This diverges from the required app header contract and will propagate everywhere AppHeader is used.
Proposed fix
<header
ref={ref}
className={cn(
- "relative z-20 shrink-0 border-b border-line bg-invert text-invert-fg",
+ "sticky top-0 z-40 shrink-0 border-b border-line bg-invert text-invert-fg",
className,
)}
{...rest}
>
<div
className={cn(
- "flex h-12 items-center justify-between px-5",
+ "flex h-14 items-center justify-between px-5",
innerClassName,
)}
>As per coding guidelines, "Header layout: use h-14, sticky top-0 z-40, with border-b".
📝 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.
| "relative z-20 shrink-0 border-b border-line bg-invert text-invert-fg", | |
| className, | |
| )} | |
| {...rest} | |
| > | |
| <div | |
| className={cn( | |
| "flex h-12 items-center justify-between px-5", | |
| innerClassName, | |
| "sticky top-0 z-40 shrink-0 border-b border-line bg-invert text-invert-fg", | |
| className, | |
| )} | |
| {...rest} | |
| > | |
| <div | |
| className={cn( | |
| "flex h-14 items-center justify-between px-5", | |
| innerClassName, |
🤖 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 `@packages/ui/src/components/app-header.tsx` around lines 95 - 103, The
AppHeader component in the outer div and inner flex container is using outdated
header layout specifications. Replace the positioning classes `relative z-20`
with `sticky top-0 z-40` in the outer className, and change the height class
from `h-12` to `h-14` in the inner flex div className. This aligns the header
with the required app header contract of sticky positioning at the top with
proper z-index and a 56px height instead of the current 48px.
Source: Coding guidelines
1 participant
Summary
AppHeadercomponent in@wikipefia/uiand use it across admin to standardize app chrome.Testing
admin,chat,library,portal, andstudio.Summary by CodeRabbit
New Features
Refactor