use custom header instead with proper links #248
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis change introduces a new customizable Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Header
participant GithubDropdownMenu
participant DropdownMenu
User->>App: Loads application
App->>Header: Renders with fluffyRepoName prop
Header->>GithubDropdownMenu: Renders with triggerButton and repo name
GithubDropdownMenu->>DropdownMenu: Initializes dropdown menu
User->>Header: Clicks GitHub dropdown button
Header->>GithubDropdownMenu: Opens dropdown
GithubDropdownMenu->>DropdownMenu: Displays menu items
User->>DropdownMenu: Selects a menu item (e.g., "Report an issue")
DropdownMenu->>User: Opens corresponding GitHub link in new tab
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
# Conflicts: # src/App.tsx # src/tailwind.css
✅ Deploy Preview for graypaper-reader ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![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
🧹 Nitpick comments (8)
src/tailwind.css (1)
4-6: Consider moving.text-brand-darkinto Tailwind theme
Definingtext-brand-darkviatailwind.config.tsundertheme.extend.colorswould be more idiomatic and allow usage of standardtext-brand-darkutilities without custom CSS.src/components/Header/Header.tsx (2)
24-25: Conflicting responsive classes – element is both hidden and flex
max-sm:flex-col-reverse max-sm:hiddencan’t coexist: once hidden the flex rule never applies.
Drop one of them (usually thehidden) or move the orientation change to a larger breakpoint.- <div className="flex max-sm:flex-col-reverse max-sm:hidden items-end md:items-center h-[50px] gap-2"> + <div className="flex sm:flex-col-reverse items-end md:items-center h-[50px] gap-2">
14-40: Add semantic & a11y tweaks to headerConsider:
- Giving the wrapper
role="banner"for landmark navigation.- Making the home link keyboard-labelled (
aria-label="Home").- Supplying a more descriptive
altfortoolNameSrc(e.g. the actual product name).These small changes improve screen-reader experience at almost zero cost.
src/components/Header/components/github-dropdown-menu.tsx (1)
50-56: Preserve caller-suppliedclassNameSpreading
...propsbefore a hard-codedclassNameoverwrites any custom classes passed in, reducing reuse.- <DropdownMenuItem {...props} className="pl-3 pt-3 cursor-pointer group"> + <DropdownMenuItem className={twMerge("pl-3 pt-3 cursor-pointer group", props.className)} {...props}>
twMergekeeps Tailwind conflict resolution intact.src/components/ui/dropdown-menu.tsx (4)
90-103: Redundantcheckedprop re-assignment
checkedis de-structured and then immediately re-injected withchecked={checked}before the final{...props}spread.
Becausecheckedmay already be present insideprops, this explicit re-assignment is unnecessary and can lead to accidental shadowing or confusion about which value wins in case the caller passes its ownchecked.- React.ComponentPropsWithoutRef<typeof DropdownMenuPrimitive.CheckboxItem> ->(({ className, children, checked, ...props }, ref) => ( + React.ComponentPropsWithoutRef<typeof DropdownMenuPrimitive.CheckboxItem> +>(({ className, children, ...props }, ref) => ( ... - checked={checked} - {...props} + {...props}The same clean-up can be applied to the
RadioItemwrapper below for consistency.
120-129: Duplicate pattern inRadioItemAfter removing the explicit
checkedin the previous block, mirror the change here by removing the redundant spread order (...propsalready carrieschecked). Keeps both wrappers symmetric and avoids future drift.
26-34: Minor sizing / radius inconsistency
DropdownMenuSubTriggerusesrounded-smwhereasDropdownMenuItemusesrounded-xl.
Unless the bigger radius is deliberate, consider standardising both to keep hover rectangles visually aligned when the cursor moves between trigger and normal items.
157-160: Export shortcut component with explicit type
DropdownMenuShortcutis currently an anonymous arrow function assigned to aconst.
For better IntelliSense and refactoring safety consider adding an explicit function component type:-const DropdownMenuShortcut = ({ className, ...props }: React.HTMLAttributes<HTMLSpanElement>) => { +const DropdownMenuShortcut: React.FC<React.HTMLAttributes<HTMLSpanElement>> = ({ + className, + ...props +}) => {Not strictly required, but it helps consumer projects that rely on automatic prop inference.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/fluffy-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (11)
package.json(1 hunks)src/App.tsx(2 hunks)src/components/Header/Header.tsx(1 hunks)src/components/Header/components/dropdown-button.tsx(1 hunks)src/components/Header/components/github-dropdown-menu.tsx(1 hunks)src/components/Header/index.ts(1 hunks)src/components/ui/button/button.tsx(1 hunks)src/components/ui/button/index.ts(1 hunks)src/components/ui/button/variants.tsx(1 hunks)src/components/ui/dropdown-menu.tsx(1 hunks)src/tailwind.css(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/components/Header/components/dropdown-button.tsx (1)
src/components/ui/button/button.tsx (2)
ButtonProps(8-12)Button(14-19)
src/components/Header/Header.tsx (2)
src/components/Header/components/github-dropdown-menu.tsx (1)
GithubDropdownMenu(12-47)src/components/Header/components/dropdown-button.tsx (1)
DropdownButton(4-12)
src/components/ui/button/button.tsx (1)
src/components/ui/button/variants.tsx (1)
buttonVariants(3-30)
src/components/Header/components/github-dropdown-menu.tsx (1)
src/components/ui/dropdown-menu.tsx (5)
DropdownMenu(163-163)DropdownMenuTrigger(164-164)DropdownMenuPortal(173-173)DropdownMenuContent(165-165)DropdownMenuItem(166-166)
🪛 GitHub Actions: Node.js CI
src/App.tsx
[error] 9-19: Biome formatting check failed: File content differs from formatting output. Run 'biome format' or equivalent to fix code style issues.
🔇 Additional comments (9)
src/components/ui/button/variants.tsx (5)
1-2: Import ofcvais correct
Thecvafunction is properly imported fromclass-variance-authority.
3-4: Base button styles defined clearly
The base utility classes cover layout, typography, focus and disabled states as intended. No issues found.
6-17: Clarify overlapping border classes inoutlineBrandvariant
TheoutlineBrandvariant includes bothborder-inputandborder-brand-dark, which may conflict or be redundant. Please confirm the intended styling order and remove any unneeded classes.
18-23: Verify icon size variant alignment
Theiconsize uses-mt-2; ensure this negative margin achieves the desired vertical alignment across button contexts.
25-28: Default variants correctly set
The defaultvariant: "default"andsize: "default"align with expected button behavior.package.json (1)
23-24: To verify that the Radix UI packages are actually in use, let’s rerun with the correctrgsyntax and confirm installation:#!/bin/bash # Check that the packages are installed at top level npm ls @radix-ui/react-dropdown-menu @radix-ui/react-popover --depth=0 # Search all source files for imports or usage rg "@radix-ui/react-dropdown-menu" -n src rg "@radix-ui/react-popover" -n srcsrc/components/Header/index.ts (1)
1-1: Re-exportHeaderfor ergonomic imports
ExposingHeaderand its props at this entry point improves module ergonomics.src/tailwind.css (1)
2-2: Confirm Tailwind configuration for@custom-variantusage
The@custom-variant darkdirective requires the Tailwind JIT configuration to support custom variants. Please ensure yourtailwind.config.tsis set up accordingly.src/components/ui/button/index.ts (1)
1-1: Add button entry-point exports
Re-exportingButtonand its props here is straightforward and aligns with the new component structure.
src/App.tsx
Outdated
| import { LightDarkThemeSyncer } from "./components/LightDarkThemeSyncer"; | ||
| import { LightThemeToggle } from "./components/LightThemeToggle/LightThemeToggle"; | ||
| import { type ILocationContext, LocationContext } from "./components/LocationProvider/LocationProvider"; | ||
| import { type IMetadataContext, MetadataContext } from "./components/MetadataProvider/MetadataProvider"; | ||
| import { | ||
| type ILocationContext, | ||
| LocationContext, | ||
| } from "./components/LocationProvider/LocationProvider"; | ||
| import { | ||
| type IMetadataContext, | ||
| MetadataContext, | ||
| } from "./components/MetadataProvider/MetadataProvider"; |
There was a problem hiding this comment.
Fix formatting to unblock CI
Biome is failing on this import block.
Run your formatter (or pnpm biome format / biome format) to normalise the multiline import { … } statements – e.g. collapse them onto one line or follow the repo’s preferred wrapped-indent style.
Until this is fixed the Node.js CI job will stay red.
🧰 Tools
🪛 GitHub Actions: Node.js CI
[error] 9-19: Biome formatting check failed: File content differs from formatting output. Run 'biome format' or equivalent to fix code style issues.
🤖 Prompt for AI Agents
In src/App.tsx around lines 9 to 18, the multiline import statements are not
formatted according to the project's style, causing the Biome formatter to fail
and blocking CI. Run the project's formatter using `pnpm biome format` or `biome
format` to normalize these import statements, either collapsing them onto one
line or applying the repo's preferred wrapped-indent style to fix the formatting
and unblock the CI.
| export const DropdownButton = (props: ButtonProps) => { | ||
| const { children, ...rest } = props; | ||
| return ( | ||
| <Button variant="outlineBrand" {...rest}> | ||
| <span>Github</span> | ||
| <LucideChevronDown /> | ||
| </Button> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused children prop & set explicit type="button"
children is destructured but ignored, so passing it silently does nothing and may confuse users.
Also, without an explicit type, a <button> defaults to "submit" inside forms, which can trigger unwanted submits.
-export const DropdownButton = (props: ButtonProps) => {
- const { children, ...rest } = props;
+export const DropdownButton = ({ children: _unused, ...rest }: ButtonProps) => {
return (
- <Button variant="outlineBrand" {...rest}>
+ <Button variant="outlineBrand" type="button" {...rest}>
<span>Github</span>
<LucideChevronDown />
</Button>
);
};📝 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.
| export const DropdownButton = (props: ButtonProps) => { | |
| const { children, ...rest } = props; | |
| return ( | |
| <Button variant="outlineBrand" {...rest}> | |
| <span>Github</span> | |
| <LucideChevronDown /> | |
| </Button> | |
| ); | |
| }; | |
| export const DropdownButton = ({ children: _unused, ...rest }: ButtonProps) => { | |
| return ( | |
| <Button variant="outlineBrand" type="button" {...rest}> | |
| <span>Github</span> | |
| <LucideChevronDown /> | |
| </Button> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In src/components/Header/components/dropdown-button.tsx around lines 4 to 12,
remove the unused destructured `children` prop from the component since it is
not used and may confuse users. Additionally, add an explicit `type="button"`
attribute to the rendered `<Button>` element to prevent it from defaulting to
`"submit"` inside forms and causing unintended form submissions.
| ({ className, variant, size, asChild = false, ...props }, ref) => { | ||
| const Comp = asChild ? Slot : "button"; | ||
| return <Comp className={twMerge(buttonVariants({ variant, size, className }))} ref={ref} {...props} />; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Default button type should be "button"
When asChild is false we render a native <button> without a type, which defaults to "submit".
Inside any form this can cause accidental submissions.
- const Comp = asChild ? Slot : "button";
- return <Comp className={twMerge(buttonVariants({ variant, size, className }))} ref={ref} {...props} />;
+ const Comp = asChild ? Slot : "button";
+ return (
+ <Comp
+ className={twMerge(buttonVariants({ variant, size, className }))}
+ ref={ref}
+ type={props.type ?? (asChild ? undefined : "button")}
+ {...props}
+ />
+ );📝 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.
| ({ className, variant, size, asChild = false, ...props }, ref) => { | |
| const Comp = asChild ? Slot : "button"; | |
| return <Comp className={twMerge(buttonVariants({ variant, size, className }))} ref={ref} {...props} />; | |
| }, | |
| ({ className, variant, size, asChild = false, ...props }, ref) => { | |
| const Comp = asChild ? Slot : "button"; | |
| return ( | |
| <Comp | |
| className={twMerge(buttonVariants({ variant, size, className }))} | |
| ref={ref} | |
| type={props.type ?? (asChild ? undefined : "button")} | |
| {...props} | |
| /> | |
| ); | |
| }, |
🤖 Prompt for AI Agents
In src/components/ui/button/button.tsx around lines 15 to 18, the native button
rendered when asChild is false lacks an explicit type attribute, causing it to
default to "submit" and potentially trigger unintended form submissions. To fix
this, add a default type="button" attribute to the button element when asChild
is false, ensuring it does not submit forms accidentally.
| onSelect={() => window.open(`${fluffyRepoUrl}/issues/new`, "_blank")} | ||
| textPrimary="Report an issue or suggestion" | ||
| textSecondary="Go to the issue creation page" | ||
| /> | ||
|
|
||
| <CustomDropdownItem | ||
| onSelect={() => window.open(`https://github.com/FluffyLabs/${fluffyRepoName}`, "_blank")} | ||
| textPrimary="Star us on Github to show support" | ||
| textSecondary="Visit our Github" | ||
| /> | ||
|
|
||
| <CustomDropdownItem | ||
| onSelect={() => window.open(`https://github.com/FluffyLabs/${fluffyRepoName}/fork`, "_blank")} | ||
| textPrimary="Fork & contribute" | ||
| textSecondary="Opens the fork creation page" | ||
| /> |
There was a problem hiding this comment.
Open links safely – prevent reverse-tabnabbing
window.open(url, "_blank") keeps a handle to window.opener.
Attackers can navigate the opener and perform phishing (“reverse tab-nabbing”).
Add "noopener,noreferrer" in the features param:
- onSelect={() => window.open(`${fluffyRepoUrl}/issues/new`, "_blank")}
+ onSelect={() => window.open(`${fluffyRepoUrl}/issues/new`, "_blank", "noopener,noreferrer")}(Apply to the other two window.open calls as well.)
📝 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.
| onSelect={() => window.open(`${fluffyRepoUrl}/issues/new`, "_blank")} | |
| textPrimary="Report an issue or suggestion" | |
| textSecondary="Go to the issue creation page" | |
| /> | |
| <CustomDropdownItem | |
| onSelect={() => window.open(`https://github.com/FluffyLabs/${fluffyRepoName}`, "_blank")} | |
| textPrimary="Star us on Github to show support" | |
| textSecondary="Visit our Github" | |
| /> | |
| <CustomDropdownItem | |
| onSelect={() => window.open(`https://github.com/FluffyLabs/${fluffyRepoName}/fork`, "_blank")} | |
| textPrimary="Fork & contribute" | |
| textSecondary="Opens the fork creation page" | |
| /> | |
| <CustomDropdownItem | |
| onSelect={() => window.open(`${fluffyRepoUrl}/issues/new`, "_blank", "noopener,noreferrer")} | |
| textPrimary="Report an issue or suggestion" | |
| textSecondary="Go to the issue creation page" | |
| /> | |
| <CustomDropdownItem | |
| onSelect={() => window.open(`https://github.com/FluffyLabs/${fluffyRepoName}`, "_blank", "noopener,noreferrer")} | |
| textPrimary="Star us on Github to show support" | |
| textSecondary="Visit our Github" | |
| /> | |
| <CustomDropdownItem | |
| onSelect={() => window.open(`https://github.com/FluffyLabs/${fluffyRepoName}/fork`, "_blank", "noopener,noreferrer")} | |
| textPrimary="Fork & contribute" | |
| textSecondary="Opens the fork creation page" | |
| /> |
🤖 Prompt for AI Agents
In src/components/Header/components/github-dropdown-menu.tsx between lines 27
and 42, the window.open calls use "_blank" without specifying
"noopener,noreferrer" in the features parameter, which can lead to reverse
tab-nabbing security issues. To fix this, update each window.open call to
include "noopener,noreferrer" as the third argument, for example
window.open(url, "_blank", "noopener,noreferrer"), to prevent the new tab from
having access to the opener window.
1 participant
No description provided.