Conversation
javiert-okta
had a problem deploying
to
preview
GitHub Actions
Failure
|
Preview URL https://openidconnect-7a9q0bk3v-okta.vercel.app |
| @@ -0,0 +1,3 @@ | |||
| export const DEFAULT_THEME = "dark"; | |||
There was a problem hiding this comment.
PR description says "default theme is light" but code sets DEFAULT_THEME = "dark"
There was a problem hiding this comment.
Yes, default should be light.
| [closeMobileMenu] | ||
| ); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
Is this useEffect still needed, all code within is commented out.
There was a problem hiding this comment.
It is needed for mobile navigation and to automatically close the menu. I have uncommented it.
| import { cookies } from "next/headers"; | ||
| import { DEFAULT_THEME, PREFERRED_THEME_NAME, THEMES } from "../theme.config"; | ||
|
|
||
| export const getThemeFromCookies = async (): Promise<string> => { |
There was a problem hiding this comment.
This whole service does not seem to be used in the codebase.
There was a problem hiding this comment.
I have deleted it
| @@ -0,0 +1,30 @@ | |||
| type LinkInfo = { | |||
There was a problem hiding this comment.
Missing trailing semicolons:
type LinkInfo = {
id: string;
label: string;
pathname: string;
isExternal?: boolean;
}
| } | ||
| } | ||
|
|
||
| .burgerIconWrapper { |
There was a problem hiding this comment.
Some classnames are in camelCase, some are BEM-style. Can you choose one style and make this consistent?
|
Preview URL https://openidconnect-3gcwoa09a-okta.vercel.app |
2 participants
By submitting a PR to this repository, you agree to the terms within the Auth0 Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.
Description
The following PR adds the header component to the site (following the new design on Figma). This change includes:
clsxpackage to include classnames on the components on a structured wayScreenshots
Desktop:
Mobile:
Desktop:
Mobile:
References
Link to Figma design
Testing
Checklist