-
Notifications
You must be signed in to change notification settings - Fork 987
fix(site): standardize headers for Admin Settings page #16911
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
Changes from 1 commit
de69ee3
a1da1c1
e5d581b
9b4928b
a35a9ae
9872c5d
1fe0022
7d0903a
a512bf5
734f59d
49b9dfe
840d6df
f169c4e
2325982
99c2dbc
50eddd0
2b77784
e2523cb
7f53666
daf6da9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,20 +4,22 @@ import { Stack } from "components/Stack/Stack"; | |
import { SquareArrowOutUpRightIcon } from "lucide-react"; | ||
import type { FC, ReactNode } from "react"; | ||
|
||
interface HeaderProps { | ||
type HeaderHierarchy = "primary" | "secondary"; | ||
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. Made it like this so that we can add additional variants over time 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. For creating variants, I would recommend you to use 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. Yeah, I've used CVA before – I actually used it in the take-home that got me into Coder. I thought it was a bit too much overhead here, but if we're switching to CVA as a go-to, I can swap that in 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. Because of shadcn, our base components are using CVA so I think it would be nice to use it but it is up to you. |
||
|
||
type HeaderProps = Readonly<{ | ||
title: ReactNode; | ||
description?: ReactNode; | ||
secondary?: boolean; | ||
hierarchy?: HeaderHierarchy; | ||
docsHref?: string; | ||
tooltip?: ReactNode; | ||
} | ||
}>; | ||
|
||
export const SettingsHeader: FC<HeaderProps> = ({ | ||
title, | ||
description, | ||
docsHref, | ||
secondary, | ||
tooltip, | ||
hierarchy = "primary", | ||
}) => { | ||
const theme = useTheme(); | ||
|
||
|
@@ -37,7 +39,7 @@ export const SettingsHeader: FC<HeaderProps> = ({ | |
marginBottom: 4, | ||
gap: 8, | ||
}, | ||
secondary && { | ||
hierarchy === "secondary" && { | ||
fontSize: 24, | ||
fontWeight: 500, | ||
}, | ||
|
@@ -60,7 +62,6 @@ export const SettingsHeader: FC<HeaderProps> = ({ | |
</span> | ||
)} | ||
</div> | ||
|
||
{docsHref && ( | ||
<Button asChild variant="outline"> | ||
<a href={docsHref} target="_blank" rel="noreferrer"> | ||
|
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.
Can we add a storybook for it covering the new variants?