-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Update shadcn/ui components to latest version and fix dialog backdrop blur #2014
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
Changes from 9 commits
bf55f60
3517535
01a56ab
3ae83f2
31b2fb3
97e98b0
7de672d
52fb6e6
501cd82
62ace3a
16a79e3
d186b98
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,6 +77,7 @@ | |
| --subtleBlack: #1F1F1F; | ||
| --skyBlue: #0066FF; | ||
| --shinyGray: #A1A1A1; | ||
| --sidebar: hsl(0 0% 98%); | ||
| } | ||
|
|
||
| .dark { | ||
|
|
@@ -114,6 +115,7 @@ | |
| --sidebar-border: hsl(240 3.7% 15.9%); | ||
| --sidebar-ring: hsl(217.2 91.2% 59.8%); | ||
| --panel: hsl(240 3.7% 10.2%); | ||
| --sidebar: hsl(240 5.9% 10%); | ||
| } | ||
|
|
||
| @theme inline { | ||
|
|
@@ -137,14 +139,14 @@ | |
| --color-border: var(--border); | ||
| --color-input: var(--input); | ||
| --color-ring: var(--ring); | ||
|
|
||
| /* Chart colors */ | ||
| --color-chart-1: var(--chart-1); | ||
| --color-chart-2: var(--chart-2); | ||
| --color-chart-3: var(--chart-3); | ||
| --color-chart-4: var(--chart-4); | ||
| --color-chart-5: var(--chart-5); | ||
|
|
||
| /* Static colors */ | ||
| --color-darkBackground: var(--darkBackground); | ||
| --color-lightBackground: var(--lightBackground); | ||
|
|
@@ -160,7 +162,7 @@ | |
| --color-subtleBlack: var(--subtleBlack); | ||
| --color-skyBlue: var(--skyBlue); | ||
| --color-shinyGray: var(--shinyGray); | ||
|
|
||
| /* sidebar colors */ | ||
| --color-sidebar: var(--sidebar-background); | ||
| --color-sidebar-foreground: var(--sidebar-foreground); | ||
|
|
@@ -170,7 +172,7 @@ | |
| --color-sidebar-accent-foreground: var(--sidebar-accent-foreground); | ||
| --color-sidebar-border: var(--sidebar-border); | ||
| --color-sidebar-ring: var(--sidebar-ring); | ||
|
|
||
| /* Border radius */ | ||
| --radius-lg: var(--radius); | ||
| --radius-md: calc(var(--radius) - 2px); | ||
|
|
@@ -234,7 +236,6 @@ | |
| --animate-blink: blink 0.8s ease-in-out infinite; | ||
| } | ||
|
|
||
|
|
||
| /* Add scrollbar styling */ | ||
| @utility style-scrollbar { | ||
| @apply scrollbar scrollbar-w-0 scrollbar-thumb-accent/40 scrollbar-track-transparent hover:scrollbar-thumb-accent scrollbar-thumb-rounded-full; | ||
|
|
@@ -473,3 +474,14 @@ | |
| -ms-overflow-style: none; /* IE and Edge */ | ||
| scrollbar-width: none; /* Firefox */ | ||
| } | ||
| /* | ||
| ---break--- | ||
| */ | ||
| @layer base { | ||
| * { | ||
| @apply border-border outline-ring/50; | ||
| } | ||
| body { | ||
| @apply bg-background text-foreground; | ||
| } | ||
| } | ||
|
Comment on lines
+480
to
+487
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainKill the duplication: two base layers fight; last one wins. You now have two Apply this diff to consolidate: @@
-@layer base {
- * {
- @apply border-border;
- }
- body {
- @apply bg-background text-foreground font-sans overscroll-none style-scrollbar;
- }
-}Optionally, if you still want the scrollbar styling and font family, fold them into the final block instead: @layer base {
* {
- @apply border-border outline-ring/50;
+ @apply border-border outline-ring/50;
}
body {
- @apply bg-background text-foreground;
+ @apply bg-background text-foreground font-sans style-scrollbar;
+ /* If you intended to drop overscroll-none, omit it; else add it back explicitly */
+ /* @apply overscroll-none; */
}
}Also sanity-check 🏁 Script executed: #!/bin/bash
echo "Occurrences of '@layer base' in apps/mail/app/globals.css:"
rg -n '@layer base' apps/mail/app/globals.css
echo ""
echo "Snippets around each occurrence:"
for L in $(rg -n '@layer base' apps/mail/app/globals.css | cut -d: -f1); do
echo "=== Snippet around line $L ==="
sed -n "$((L-5)),$((L+10))p" apps/mail/app/globals.css
echo ""
doneLength of output: 1199 Consolidate duplicate 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -72,10 +72,7 @@ export function OnboardingDialog({ | |||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||
| <Dialog open={open} onOpenChange={onOpenChange}> | ||||||||||||||||||||||||||||||
| <DialogTitle></DialogTitle> | ||||||||||||||||||||||||||||||
| <DialogContent | ||||||||||||||||||||||||||||||
| showOverlay | ||||||||||||||||||||||||||||||
| className="bg-panelLight mx-auto w-full max-w-[90%] rounded-xl border p-0 sm:max-w-[690px] dark:bg-[#111111]" | ||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||
| <DialogContent className="bg-panelLight mx-auto w-full max-w-[90%] rounded-xl border p-0 sm:max-w-[690px] dark:bg-[#111111]"> | ||||||||||||||||||||||||||||||
| <div className="flex flex-col gap-4 p-4"> | ||||||||||||||||||||||||||||||
|
Comment on lines
+75
to
76
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Place DialogTitle inside DialogContent (or add a header) for proper a11y semantics. Right now DialogTitle sits outside Content. Move it inside so aria attributes resolve correctly. - <Dialog open={open} onOpenChange={onOpenChange}>
- <DialogTitle></DialogTitle>
- <DialogContent className="bg-panelLight mx-auto w-full max-w-[90%] rounded-xl border p-0 sm:max-w-[690px] dark:bg-[#111111]">
+ <Dialog open={open} onOpenChange={onOpenChange}>
+ <DialogContent className="bg-panelLight mx-auto w-full max-w-[90%] rounded-xl border p-0 sm:max-w-[690px] dark:bg-[#111111]">
+ {/* Optional header for a11y */}
+ {/* <DialogHeader><DialogTitle>Welcome to Zero Email</DialogTitle></DialogHeader> */}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
| {steps[currentStep] && steps[currentStep].video && ( | ||||||||||||||||||||||||||||||
| <div className="relative flex items-center justify-center"> | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ import { | |
| DialogTitle, | ||
| } from '@/components/ui/dialog'; | ||
| import { Drawer, DrawerContent, DrawerHeader, DrawerTitle } from '@/components/ui/drawer'; | ||
| import { VisuallyHidden } from 'radix-ui'; | ||
| import { VisuallyHidden } from '@radix-ui/react-visually-hidden'; | ||
| import { type ReactElement } from 'react'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Type import nit — import ReactNode directly. Cleaner and consistent with TS strict prefs. -import { type ReactElement } from 'react';
+import type { ReactElement, ReactNode } from 'react';
...
type ResponsiveModalProps = {
- children: React.ReactNode;
+ children: ReactNode;
open: boolean;
onOpenChange: (open: boolean) => void;
};Also applies to: 13-17 🤖 Prompt for AI Agents |
||
| // import { useMedia } from 'react-use'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) isDesktop hardcoded to true — mobile Drawer path is dead. Intended? If not deliberate, re-enable a media query so this actually responds. Speed is key; correctness too. -// import { useMedia } from 'react-use';
+import { useMedia } from 'react-use';
...
- const isDesktop = true;
+ const isDesktop = useMedia('(min-width: 768px)', true);Verify no hydration warnings and that Drawer renders below 768px. Also applies to: 24-27 🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,21 +4,16 @@ import * as React from 'react'; | |||||||||||||
| import { cn } from '@/lib/utils'; | ||||||||||||||
|
|
||||||||||||||
| const badgeVariants = cva( | ||||||||||||||
| 'inline-flex items-center rounded-full border px-2.5 py-0.5 text-xs font-semibold focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2', | ||||||||||||||
| 'inline-flex items-center rounded-full border px-2.5 py-0.5 text-xs font-semibold transition-colors focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2', | ||||||||||||||
| { | ||||||||||||||
|
Comment on lines
+7
to
8
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) 🛠️ Refactor suggestion Replace focus:outline-none → focus-visible:outline-hidden and wrap to ≤100 chars. - 'inline-flex items-center rounded-full border px-2.5 py-0.5 text-xs font-semibold transition-colors focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2',
+ 'inline-flex items-center rounded-full border px-2.5 py-0.5 text-xs font-semibold',
+ 'transition-colors focus-visible:outline-hidden focus-visible:ring-2',
+ 'focus-visible:ring-ring focus-visible:ring-offset-2',📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| variants: { | ||||||||||||||
| variant: { | ||||||||||||||
| default: 'border-transparent bg-primary text-primary-foreground ', | ||||||||||||||
| default: 'border-transparent bg-primary text-primary-foreground hover:bg-primary/80', | ||||||||||||||
| secondary: | ||||||||||||||
| 'border-transparent bg-secondary text-secondary-foreground hover:bg-secondary/80', | ||||||||||||||
| destructive: | ||||||||||||||
| 'border-transparent bg-destructive text-destructive-foreground hover:bg-destructive/80', | ||||||||||||||
| outline: 'text-foreground', | ||||||||||||||
| important: 'border-0 text-amber-700 dark:text-amber-500 ', | ||||||||||||||
| promotions: 'border-0 text-red-700 dark:text-red-500 ', | ||||||||||||||
| personal: 'border-0 text-green-700 dark:text-green-500 ', | ||||||||||||||
| updates: 'border-0 text-purple-700 dark:text-purple-500 ', | ||||||||||||||
| forums: 'border-transparent text-blue-500 dark:text-blue-400', | ||||||||||||||
| }, | ||||||||||||||
| }, | ||||||||||||||
| defaultVariants: { | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Unify the sidebar token — single source of truth.
You introduced
--sidebarbut the theme still maps to--sidebar-background. Wire it up or delete the new var to avoid dead config.Apply this diff to use the new token across the theme:
If other files still consume
--sidebar-background, keep both and plan a follow-up sweep.Also applies to: 118-118, 166-174
🤖 Prompt for AI Agents