-
Notifications
You must be signed in to change notification settings - Fork 1
Theming updates #346
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: develop
Are you sure you want to change the base?
Theming updates #346
Changes from all commits
ff0faa4
bd3ed44
503a72c
2e72afa
0d6a66c
bf8d28f
4f1e14a
6255d66
5b285a1
1c91088
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 |
|---|---|---|
|
|
@@ -31,20 +31,35 @@ const { header, footer } = branding; | |
|
|
||
| const { font_header: fontHeader, font_body: fontBody } = branding; | ||
|
|
||
| const colorPrimary = branding.primary_color; | ||
| const textPrimary = useBlackText(colorPrimary) ? 'var(--color-black)' : 'var(--color-white)'; | ||
| //dark and light mode default text color | ||
| const content = branding.content_color || 'var(--color-black)'; | ||
| const contentInverse = branding.content_inverse || 'var(--color-white)'; | ||
| const contentAlternate = branding.content_alternate || 'var(--color-neutral-dark)'; | ||
| const contentInverseAlternate = branding.content_inverse_alternate || 'var(--color-neutral-light)'; | ||
|
|
||
| const colorPrimary = branding.primary_color; | ||
| const colorSecondary = branding.secondary_color; | ||
| const textSecondary = !colorSecondary || useBlackText(colorSecondary) ? 'var(--color-black)' : 'var(--color-white)'; | ||
| const colorTertiary = branding.tertiary_color || 'var(--color-black)'; | ||
|
|
||
| //these are for use in layout components like header and footer | ||
| const primaryContent = useBlackText(colorPrimary) ? content : contentInverse; | ||
| const secondaryContent = !colorSecondary || useBlackText(colorSecondary) ? content : contentInverse; | ||
| const tertiaryContent = !colorTertiary || useBlackText(colorTertiary) ? content : contentInverse; | ||
|
|
||
| const colorLayout = branding.background_color || 'var(--color-white)'; | ||
| const layoutContent = !branding.background_color || useBlackText(branding.background_color) ? content : contentInverse; | ||
| const layoutContentAlternate = !branding.background_color || useBlackText(branding.background_color) ? contentAlternate : contentInverseAlternate; | ||
|
|
||
| const colorLayoutAlternate = branding.background_alternate || 'var(--color-neutral-light)'; | ||
| const layoutAlternateContent = !branding.background_alternate || useBlackText(branding.background_alternate) ? content : contentInverse; | ||
| const layoutAlternateContentAlternate = !branding.background_alternate || useBlackText(branding.background_alternate) ? contentAlternate : contentInverseAlternate; //this naming scheme could be better.... | ||
|
|
||
| const currentLocale = Astro.currentLocale; | ||
| const lang = getLanguageFromUrl(Astro.url.pathname); | ||
|
|
||
| const pages = await getPages(currentLocale); | ||
|
|
||
| const { footer: includeFooter, fullscreen, name, title, t, tab } = Astro.props; | ||
| const { footer: includeFooter, fullscreen, name, title, t, tab, transparent } = Astro.props; | ||
|
|
||
| const NavKeys = { | ||
| gallery: 'gallery', | ||
|
|
@@ -132,11 +147,20 @@ if (config.gallery) { | |
| define:vars={{ | ||
| customFontHeader: fontHeader, | ||
| customFontBody: fontBody, | ||
| customLayout: colorLayout, | ||
| customLayoutAlternate: colorLayoutAlternate, | ||
| customColorPrimary: colorPrimary, | ||
| customColorSecondary: colorSecondary, | ||
| customTextPrimary: textPrimary, | ||
| customTextSecondary: textSecondary, | ||
| customColorLayout: colorLayout | ||
| customColorTertiary: colorTertiary, | ||
| customPrimaryContent: primaryContent, | ||
| customSecondaryContent: secondaryContent, | ||
| customTertiaryContent: tertiaryContent, | ||
| customContent: layoutContent, | ||
| customContentAlternate: layoutContentAlternate, | ||
|
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. These names are a little confusing (content alternate, alternate content, etc). Could we keep the naming more consistent with the fields in Tina? (e.g. main background, alternate background, main text, main text inverted, etc)
Contributor
Author
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 it's tricky... I was attempting to match the language that's used in the Figma designs as much as possible, which is why I switched to The thing about "main text", "main text inverted" etc. is that those are not the same variables as these; they are the options from which these theme variables are selected. For example, imagine a site with the following theme: Primary: light pink In this case (with the current naming), we'd have The reason for doing it this way is that in some cases in the designs we want to select the text color just based on whether it's dark or light (for example on these cards -- this isn't based on a theme color, we're just applying a black gradient over the image to make the light text pop:) Sorry for the overly labored explanation, but that's the complexity I'm trying to wrangle here. The user never has to see any of this -- from the perspective of an editor in Tina, they just have to select the text color they want for dark backgrounds and for light backgrounds, and the correct one will be chosen based on the background color for each section.
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. Fair enough. It's a complicated concept. I'm just thinking of another developer (myself included) coming in here to make adjustments, it's a little hard to follow which colors are being applied where. |
||
| customAlternateContent: layoutAlternateContent, | ||
| customAlternateContentAlternate: layoutAlternateContentAlternate, | ||
| customText: content, | ||
| customTextInverse: contentInverse | ||
| }} | ||
| > | ||
| </style> | ||
|
|
@@ -153,6 +177,7 @@ if (config.gallery) { | |
| items={NavItems} | ||
| pages={pages} | ||
| title={header?.hide_title ? undefined : branding.title} | ||
| transparent={transparent} | ||
| /> | ||
| <NotificationPanel | ||
| client:only='react' | ||
|
|
||

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.
Single quotes.