fix(ui): Remove Content Layout and move userinfo to global state
Context
Currently, the use(getUserInfo()) call is placed outside the <Suspense> boundary in the ContentLayout component. This causes rendering to be blocked until data is resolved, and the Suspense fallback (<SkeletonContentLayout />) is never shown. Objective: Centralize page layout management by moving title/icon definitions and navbar functionality from individual pages into the main layout component.
What Changed: ✅ Centralized Metadata: Created PAGE_METADATA array in main-layout.tsx mapping all routes to titles/icons ✅ Unified Layout: Moved navbar rendering into main layout using usePathname() for dynamic content ✅ Simplified Pages: Removed ContentLayout wrapper from 13+ pages - they now render content directly ✅ Cleaner Architecture: Single source of truth for page metadata, reduced code duplication
Description
DRY Principle: No more repeating title/icon on every page Easier Maintenance: Adding new pages only requires updating the metadata array Better Performance: Single layout renders navbar once vs. per-page rendering Cleaner Code: Pages focus purely on content without layout concerns
Fix: Result: All pages now automatically get proper titles, icons, and navbar through the centralized main layout system.
Checklist
- Are there new checks included in this PR? Yes / No
- If so, do we need to update permissions for the provider? Please review this carefully.
- [ ] Review if the code is being covered by tests.
- [ ] Review if code is being documented following this specification https://github.com/google/styleguide/blob/gh-pages/pyguide.md#38-comments-and-docstrings
- [ ] Review if backport is needed.
- [ ] Review if is needed to change the Readme.md
- [ ] Ensure new entries are added to CHANGELOG.md, if applicable.
API
- [ ] Verify if API specs need to be regenerated.
- [ ] Check if version updates are required (e.g., specs, Poetry, etc.).
- [ ] Ensure new entries are added to CHANGELOG.md, if applicable.
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Thanks for the catch, I think it's better to get the user info in
user-nav.tsxrather than in navbar.tsx to avoid the prop drilling becausenavbar.tsxdoes not use that data.
For some reason The suggested change is not working . I tried moving the Suspense directly on to navbar.tsx but it is showing the loading screen endlessly.
My guess is probably because the user-nav.tsx is a client component and the Suspense is in server component (navbar.tsx or content-layout.tsx).
So, I am reverting this change
Thanks for the catch, I think it's better to get the user info in
user-nav.tsxrather than in navbar.tsx to avoid the prop drilling becausenavbar.tsxdoes not use that data.For some reason The suggested change is not working . I tried moving the Suspense directly on to
navbar.tsxbut it is showing the loading screen endlessly.My guess is probably because the
user-nav.tsxis a client component and the Suspense is in server component (navbar.tsxorcontent-layout.tsx).So, I am reverting this change
Server actions can be called from client components like:
export const UserNav = () => {
const [user, setUser] = useState<UserProfileProps | null>(null);
useEffect(() => {
const fetchUser = async () => {
const user = await getUserInfo();
setUser(user);
};
fetchUser();
}, []);
if (!user || !user.data) return null;
We have to choose whether to pass it from the father or call it in the client component.
https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions-and-mutations#behavior
Thanks for the catch, I think it's better to get the user info in
user-nav.tsxrather than in navbar.tsx to avoid the prop drilling becausenavbar.tsxdoes not use that data.For some reason The suggested change is not working . I tried moving the Suspense directly on to
navbar.tsxbut it is showing the loading screen endlessly. My guess is probably because theuser-nav.tsxis a client component and the Suspense is in server component (navbar.tsxorcontent-layout.tsx). So, I am reverting this changeServer actions can be called from client components like:
export const UserNav = () => { const [user, setUser] = useState<UserProfileProps | null>(null); useEffect(() => { const fetchUser = async () => { const user = await getUserInfo(); setUser(user); }; fetchUser(); }, []); if (!user || !user.data) return null;We have to choose whether to pass it from the father or call it in the client component.
https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions-and-mutations#behavior
Thanks for the reference. I will look into it
The current approach is better than calling it in the user-nav for mainly two reasons:
- useEffects are not compatible with Suspense since it happens after page mount, so we would have to handle the loading state manually in the user-nav (defeating the purpose of Suspense)
- Data fetching would happen on the client, losing server-side rendering advantages."
The current approach is better than calling it in the user-nav for mainly two reasons:
- useEffects are not compatible with Suspense since it happens after page mount, so we would have to handle the loading state manually in the user-nav (defeating the purpose of Suspense)
- Data fetching would happen on the client, losing server-side rendering advantages."
- True, Suspense is easier to develop and understand.
- False, data fetching will happen on the server because getUserInfo is a server action.
The point is that there are a lot of options in NextJS apps. For me, what makes more sense is to get the user on top of the app and save it in a shared state (Zustand, for example) and then consume it where is needed.
ATM, we can get it on the ContentLayout and remove the suspense, because we need the user to render the content; otherwise, what content should we render if there is no user? Another option is what you propose, get it on the navbar, this is not a good implementation because a NavBar should just render a UI component, not get data.
It is important to be clear in separating concerns and responsibilities. navbar shouldn't get user info, should be agnostic to the user.
Then the question is, given all the options we have and the time we have to improve this, what is the better option?
Option 1: Global State (Zustand) - BEST Long-term
// store/user-store.ts
import { create } from 'zustand'
interface UserStore {
user: UserProfileProps | null
setUser: (user: UserProfileProps) => void
}
export const useUserStore = create<UserStore>((set) => ({
user: null,
setUser: (user) => set({ user }),
}))
// app/(prowler)/layout.tsx
export default async function Layout({ children }) {
const user = await getUserInfo()
return (
<UserProvider user={user}>
{children}
</UserProvider>
)
}
// components/ui/user-nav/user-nav.tsx
export const UserNav = () => {
const user = useUserStore(state => state.user)
// ...
}
✅ Pros:
- Perfect separation of concerns
- Single call per session
- Scalable for entire app
- Navbar completely agnostic
❌ Cons:
- Requires more initial setup
- Needs provider in layout
- More architectural complexity
Option 2: ContentLayout async (no Suspense) - BEST for Limited Time
// content-layout.tsx
export async function ContentLayout({ title, icon, children }) {
const user = await getUserInfo() // Blocks until user is available
return (
<>
<Navbar title={title} icon={icon} user={user} />
<div className="px-6 py-4 sm:px-8 xl:px-10">{children}</div>
</>
)
}
✅ Pros:
- Minimal change (just add async)
- Maintains separation of concerns
- Guarantees user is always available
- No regressions
❌ Cons:
- Blocks entire page if getUserInfo is slow
Option 3: Navbar async - Works but Violates Principles
// ❌ Violates Single Responsibility
export async function Navbar({ title, icon }) {
const user = await getUserInfo() // Navbar fetching data
return <header>...</header>
}
❌ Issues:
- Navbar should be presentational only
- Violates separation of concerns
- Makes component less reusable
Recommendation
For IMMEDIATE Implementation: Option 2 (ContentLayout async)
// components/ui/content-layout/content-layout.tsx
import { getUserInfo } from "@/actions/users/users";
import { Navbar } from "../nav-bar/navbar";
interface ContentLayoutProps {
title: string;
icon: string;
children: React.ReactNode;
}
export async function ContentLayout({ title, icon, children }: ContentLayoutProps) {
const user = await getUserInfo()
return (
<>
<Navbar title={title} icon={icon} user={user} />
<div className="px-6 py-4 sm:px-8 xl:px-10">{children}</div>
</>
)
}
Reasons:
- ✅ Minimal time: Just add
asyncandawait - ✅ Maintains principles: Navbar remains presentational
- ✅ Logical: Need user to determine what content to render
- ✅ No breaking changes: Doesn't affect existing code
For FUTURE Improvement: Option 1 (Global State)
What do you think @paabloLC?
The current approach is better than calling it in the user-nav for mainly two reasons:
- useEffects are not compatible with Suspense since it happens after page mount, so we would have to handle the loading state manually in the user-nav (defeating the purpose of Suspense)
- Data fetching would happen on the client, losing server-side rendering advantages."
- True, Suspense is easier to develop and understand.
- False, data fetching will happen on the server because getUserInfo is a server action.
The point is that there are a lot of options in NextJS apps. For me, what makes more sense is to get the user on top of the app and save it in a shared state (Zustand, for example) and then consume it where is needed.
ATM, we can get it on the
ContentLayoutand remove the suspense, because we need the user to render the content; otherwise, what content should we render if there is no user? Another option is what you propose, get it on the navbar, this is not a good implementation because aNavBarshould just render a UI component, not get data.It is important to be clear in separating concerns and responsibilities.
navbarshouldn't get user info, should be agnostic to the user.Then the question is, given all the options we have and the time we have to improve this, what is the better option?
Option 1: Global State (Zustand) - BEST Long-term
// store/user-store.ts import { create } from 'zustand' interface UserStore { user: UserProfileProps | null setUser: (user: UserProfileProps) => void } export const useUserStore = create<UserStore>((set) => ({ user: null, setUser: (user) => set({ user }), })) // app/(prowler)/layout.tsx export default async function Layout({ children }) { const user = await getUserInfo() return ( <UserProvider user={user}> {children} </UserProvider> ) } // components/ui/user-nav/user-nav.tsx export const UserNav = () => { const user = useUserStore(state => state.user) // ... }✅ Pros:
- Perfect separation of concerns
- Single call per session
- Scalable for entire app
- Navbar completely agnostic
❌ Cons:
- Requires more initial setup
- Needs provider in layout
- More architectural complexity
Option 2: ContentLayout async (no Suspense) - BEST for Limited Time
// content-layout.tsx export async function ContentLayout({ title, icon, children }) { const user = await getUserInfo() // Blocks until user is available return ( <> <Navbar title={title} icon={icon} user={user} /> <div className="px-6 py-4 sm:px-8 xl:px-10">{children}</div> </> ) }✅ Pros:
- Minimal change (just add async)
- Maintains separation of concerns
- Guarantees user is always available
- No regressions
❌ Cons:
- Blocks entire page if getUserInfo is slow
Option 3: Navbar async - Works but Violates Principles
// ❌ Violates Single Responsibility export async function Navbar({ title, icon }) { const user = await getUserInfo() // Navbar fetching data return <header>...</header> }❌ Issues:
- Navbar should be presentational only
- Violates separation of concerns
- Makes component less reusable
Recommendation
For IMMEDIATE Implementation: Option 2 (ContentLayout async)
// components/ui/content-layout/content-layout.tsx import { getUserInfo } from "@/actions/users/users"; import { Navbar } from "../nav-bar/navbar"; interface ContentLayoutProps { title: string; icon: string; children: React.ReactNode; } export async function ContentLayout({ title, icon, children }: ContentLayoutProps) { const user = await getUserInfo() return ( <> <Navbar title={title} icon={icon} user={user} /> <div className="px-6 py-4 sm:px-8 xl:px-10">{children}</div> </> ) }Reasons:
- ✅ Minimal time: Just add
asyncandawait- ✅ Maintains principles: Navbar remains presentational
- ✅ Logical: Need user to determine what content to render
- ✅ No breaking changes: Doesn't affect existing code
For FUTURE Improvement: Option 1 (Global State)
What do you think @paabloLC?
About the 2nd point what i mean is the api call is initiated after the page is mounted unlike the original approach, data fetching does happen on server in both cases. And I don't think there is anything wrong with api call in the navbar as there is no degradation in performance at all and we won't be reusing the navbar anyway
And I don't think there is anything wrong with api call in the navbar as there is no degradation in performance at all and we won't be reusing the navbar anyway
Is not about degradation, is about the separation of concerns, but anyway, there are a few things to change.
- Content Layout shouldn't exist. In NextJS apps, the Layout component is responsible for these layouts that are repeated across the app.
- The user should be requested at the top level of the app and saved in a global state, then we can toggle between views in the components regarding the roles of the user.
- Content Layout shouldn't exist. In NextJS apps, the Layout component is responsible for these layouts that are repeated across the app.
- The user should be requested at the top level of the app and saved in a global state, then we can toggle between views in the components regarding the roles of the user.
Yes! We can do that. I was just trying to use whatever we had for a quick fix.
- Content Layout shouldn't exist. In NextJS apps, the Layout component is responsible for these layouts that are repeated across the app.
- The user should be requested at the top level of the app and saved in a global state, then we can toggle between views in the components regarding the roles of the user.
Yes! We can do that. I was just trying to use whatever we had for a quick fix.
I appreciate your suggestion, thanks a lot.
Hi @alejandrobailo . Implemented the long term fix!
Objective: Centralize page layout management by moving title/icon definitions and navbar functionality from individual pages into the main layout component.
What Changed: ✅ Centralized Metadata: Created PAGE_METADATA array in main-layout.tsx mapping all routes to titles/icons ✅ Unified Layout: Moved navbar rendering into main layout using usePathname() for dynamic content ✅ Simplified Pages: Removed ContentLayout wrapper from 13+ pages - they now render content directly ✅ Cleaner Architecture: Single source of truth for page metadata, reduced code duplication Key Benefits:
- DRY Principle: No more repeating title/icon on every page
- Easier Maintenance: Adding new pages only requires updating the metadata array
- Better Performance: Single layout renders navbar once vs. per-page rendering
- Cleaner Code: Pages focus purely on content without layout concerns
Result: All pages now automatically get proper titles, icons, and navbar through the centralized main layout system.