Settings #56
Settings #56
Conversation
Deploying redot-website with
|
| Latest commit: |
aab4ff0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://827ee61c.redot-website.pages.dev |
| Branch Preview URL: | https://feature-settings.redot-website.pages.dev |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Added a comprehensive settings system with customizable blog layouts, featuring a new settings page with appearance customization options and persistent user preferences.
- Added
/actions/settings.tsfor managing blog layout preferences via cookies withsaveSettingsBlogLayoutandgetSettingsBlogLayoutfunctions - Implemented
AppearanceFormcomponent with visual layout previews and radio group selection between "new" and "old" blog styles - Split
ArticleSplashintoArticleSplashNewandArticleSplashOldcomponents with distinct layout styles - Added settings navigation with
SettingsSidebarcomponent and configurable sidebar items - Integrated Radix UI components (Label, RadioGroup) for enhanced form controls and accessibility
12 file(s) reviewed, 16 comment(s)
Edit PR Review Bot Settings | Greptile
| }; | ||
|
|
||
| export default function Layout({ | ||
| children, | ||
| }: Readonly<{ | ||
| children: React.ReactNode; | ||
| }>) { | ||
| return <>{children}</>; | ||
| return ( | ||
| <div className="hidden space-y-6 p-10 pb-16 md:block"> |
There was a problem hiding this comment.
logic: Hidden on mobile screens with no alternative mobile navigation provided
| return ( | ||
| <div className="hidden space-y-6 p-10 pb-16 md:block"> | ||
| <div className="space-y-0.5"> | ||
| <h2 className="text-2xl font-bold tracking-tight">Settings</h2> |
There was a problem hiding this comment.
style: Consider adding aria-label to improve accessibility since this is the main settings heading
| <p className="text-sm text-muted-foreground"> | ||
| Customize the look and feel of your experience by adjusting themes, | ||
| colors, and display preferences. | ||
| </p> |
There was a problem hiding this comment.
logic: description text implies theme customization but AppearanceForm only shows blog layout options - either update text or expand functionality
| </div> | ||
| <Separator className="my-6" /> | ||
| <div className="flex flex-col space-y-8 lg:flex-row lg:space-x-12 lg:space-y-0"> | ||
| <aside className="-mx-4 lg:w-1/5"> |
There was a problem hiding this comment.
style: Negative margin (-mx-4) could cause horizontal scrolling issues on some viewports
| <div | ||
| className="[&:has([data-state=checked])>div]:border-primary" | ||
| onClick={() => handleLayoutChange("new")} | ||
| > |
There was a problem hiding this comment.
style: redundant onClick handler since RadioGroup already handles value changes through onValueChange
| > | ||
| <RadioGroupItem value="new" className="sr-only" /> | ||
| <div className="items-center rounded-md border-2 border-muted p-1 hover:border-accent"> | ||
| <div className="space-y-2 rounded-sm bg-white p-2 shadow-sm"> |
There was a problem hiding this comment.
style: consider adding dark mode support for bg-white and shadow styling
|
PR Content:
AppearanceFormcomponent for appearance settings.ArticleSplashcomponent for better maintainability.