-
Notifications
You must be signed in to change notification settings - Fork 13
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
Fix layout #454
base: main
Are you sure you want to change the base?
Fix layout #454
Conversation
src/app/layout.tsx
Outdated
useEffect(() => { | ||
const fetchPartials = async () => { | ||
const headerResponse = await fetch(`/api/chrome/rh-universal-nav-header`); | ||
const footerResponse = await fetch(`/api/chrome/rh-unified-footer`); |
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.
could you fetch the header and footer in parallel instead of sequentially ?
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.
Sure thing
@@ -32,7 +32,8 @@ | |||
})(); | |||
</script> | |||
<script> | |||
(function (env) { | |||
(function () { |
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.
are we still using this file? for development perhaps?
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.
This file is loaded before the javascript kicks in.
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.
This file is not used. The JS does not have to kick in, the site is statically generated. Next.js creates ints own index file unless the server ins misconfigured.
|
||
location /remote { | ||
rewrite ^/remote/(.*)$ /$1 break; | ||
proxy_pass https://developers.redhat.com; |
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.
Is any of this still required?
It looks like we moved away from nginx loading to loading it from the frontend
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.
Good as a hotfix. But has to be properly fixed in a follow up.
@@ -32,7 +32,8 @@ | |||
})(); | |||
</script> | |||
<script> | |||
(function (env) { | |||
(function () { |
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.
This file is not used. The JS does not have to kick in, the site is statically generated. Next.js creates ints own index file unless the server ins misconfigured.
|
||
export default function RootLayout({ | ||
children, | ||
}: Readonly<{ | ||
children: React.ReactNode; | ||
}>) { | ||
const [header, setHeader] = useState<string | undefined>(); |
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.
We need to fix this using actual web components. This is bad. Unless the header and footer are not web components but ESI. In which case we are done for.
Description
Since we moved to Next.js we have to adjust the loading of navigation and footer. We can utilize HTML partials on developers site to load both content and JS files.