feat: sidebar#39
Open
lorenzocorallo wants to merge 28 commits into
Open
Conversation
|
Warning Review limit reached
More reviews will be available in 50 minutes and 56 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
WalkthroughThis PR replaces the top-level AdminHeader with a new collapsible sidebar navigation system, adds cookie-backed state persistence and breadcrumb derivation, composes DashboardSidebar (nav data, main-nav, user-nav), and updates layouts/pages to use the new shell while removing per-page back links and legacy header usages. ChangesSidebar Navigation Architecture
Dashboard Page Layout Refinements
Configuration & TypeScript Updates
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/dashboard/`(active)/layout.tsx:
- Around line 9-12: The layout reads a client-controlled cookie into cookie and
directly JSON.parse's it into DSCategoryState which can throw on malformed
input; wrap the parse in a safe guard (try/catch) or use a safeJsonParse helper
around the cookie value and fall back to an empty object on error so
layout.tsx’s DSCategoryState is always an object and parsing failures are
logged/ignored; update the code that references cookieStore, cookie and
DSCategoryState to use this safe parse.
In `@src/app/dashboard/`(active)/telegram/groups/page.tsx:
- Line 26: The column header text "Hide" in the groups listing is misleading
because the last column in GroupRow (component/group-row.tsx,
functions/rendering around the hide toggle and leave-chat actions) contains both
the hide toggle and leave-chat controls; update the header in page.tsx (where
the <p>Hide</p> is defined) to a more accurate label such as "Actions" or
"Controls" (or "Hide / Leave" if you prefer explicitness) and adjust any
associated aria-labels/tooltip text to match so the header aligns with the
actual controls rendered by GroupRow.
In `@src/components/dashboard-sidebar/data.tsx`:
- Around line 27-37: flattenNavigation currently only maps first-level items
from DSData so nested route titles are missing in NAV_MAP; update
flattenNavigation (and its inner traverse) to recursively walk child arrays
(e.g., items with a children or items property) and add every node with a url to
the map, ensuring NAV_MAP contains entries for nested routes used by
breadcrumb.tsx; locate flattenNavigation, DSData, traverse, and NAV_MAP and make
traverse call itself for child lists and/or child properties until all nested
nodes are added.
In `@src/components/dashboard-sidebar/main-nav.tsx`:
- Around line 21-27: DSMenuCategory currently creates its own
useCookieStorage(COOKIES.SIDEBAR_CATEGORY_STATE) per instance which causes
concurrent writes to clobber each other and also mutates state in place;
instead, move the single useCookieStorage(COOKIES.SIDEBAR_CATEGORY_STATE) call
up into DSMainNav (call it once to get shared categoryState and
setCategoryState) and pass the state and setter down to DSMenuCategory as props;
ensure DSMenuCategory updates use an immutable setter (no in-place mutation of
the state object—use a new object like {...state, [title]: newValue}) so toggles
for one category won’t overwrite others.
- Around line 77-79: The nav highlighting uses exact equality (const path =
usePathname(); const isActive = path === item.url) so nested/detail routes don't
mark the parent active; change the logic in the component that computes isActive
(where usePathname(), path and item.url are referenced) to treat the item as
active when the current path startsWith the item.url (and normalize trailing
slashes or handle the root "/" case) so parent menu items remain highlighted for
nested routes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11cd920e-0253-4982-bc61-5efb30ca7cd4
📒 Files selected for processing (30)
src/app/dashboard/(active)/account/page.tsxsrc/app/dashboard/(active)/azure/members/page.tsxsrc/app/dashboard/(active)/azure/page.tsxsrc/app/dashboard/(active)/breadcrumb.tsxsrc/app/dashboard/(active)/layout.tsxsrc/app/dashboard/(active)/page.tsxsrc/app/dashboard/(active)/telegram/grants/page.tsxsrc/app/dashboard/(active)/telegram/groups/group-row.tsxsrc/app/dashboard/(active)/telegram/groups/page.tsxsrc/app/dashboard/(active)/telegram/page.tsxsrc/app/dashboard/(active)/telegram/user-details/page.tsxsrc/app/dashboard/(active)/telegram/user-details/remove-role.tsxsrc/app/dashboard/(active)/telegram/user-list/page.tsxsrc/app/dashboard/layout.tsxsrc/app/layout.tsxsrc/app/page.tsxsrc/components/admin-header/index.tsxsrc/components/admin-header/right-nav.tsxsrc/components/dashboard-sidebar/data.tsxsrc/components/dashboard-sidebar/index.tsxsrc/components/dashboard-sidebar/main-nav.tsxsrc/components/dashboard-sidebar/user-nav.tsxsrc/components/ui/sidebar.tsxsrc/constants.tssrc/hooks/use-cookie-storage.tsxsrc/hooks/use-session-storage.tsxsrc/index.csssrc/lib/i18n.tssrc/utils/cookies.tstsconfig.json
💤 Files with no reviewable changes (4)
- src/lib/i18n.ts
- src/components/admin-header/index.tsx
- src/app/dashboard/(active)/telegram/page.tsx
- src/components/admin-header/right-nav.tsx
Member
Author
|
ci sono dei bug nella persistence degli stati open dei collapsible che devo fixare |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/app/dashboard/(active)/breadcrumb.tsx (1)
63-66: ⚡ Quick winReplace hard-coded breadcrumb index rule with route-aware link eligibility.
Line 65 hard-codes
i !== 1, which couples behavior to the current URL depth. Prefer deriving linkability fromNAV_MAPso future route changes don’t break breadcrumb behavior.Suggested diff
- breadcrumbs.push({ title, url: i !== 1 ? currentPath : undefined }) + const isNavigable = navMap.has(currentPath) + breadcrumbs.push({ title, url: isNavigable ? currentPath : undefined }) i++🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/dashboard/`(active)/breadcrumb.tsx around lines 63 - 66, The breadcrumb logic currently uses a hard-coded index check (i !== 1) when building breadcrumb objects in breadcrumb.tsx; replace this with route-aware link eligibility by consulting NAV_MAP (or the existing route metadata) instead of relying on the iteration index. Locate where breadcrumbs.push({ title, url: i !== 1 ? currentPath : undefined }) is used and change it to compute a boolean like isLinkable = NAV_MAP[currentPath]?.linkable || NAV_MAP[currentPath]?.isPage (or whatever metadata key denotes navigable pages) and then push { title, url: isLinkable ? currentPath : undefined }; remove dependence on the variable i/current depth so future route structure changes won’t break breadcrumb link behavior.src/hooks/use-cookie-storage.tsx (1)
28-46: ⚡ Quick winConsider using functional setState to stabilize the
setValuecallback.The current implementation includes
storedValueinsetValue's dependencies, causing a new callback reference on every state update. This can trigger unnecessary re-renders in components that receivesetValueas a prop, and functional updates read from the closure rather than React's state queue.♻️ Refactor to use functional setState
const setValue: Dispatch<SetStateAction<T>> = useCallback( (value) => { - try { - const valueToStore = value instanceof Function ? value(storedValue) : value - setStoredValue(valueToStore) - - if (typeof document !== "undefined") { - if (valueToStore === null || valueToStore === undefined) { - deleteCookie(key, { path: mergedOptions.path, domain: mergedOptions.domain }) - } else { - setCookie(key, JSON.stringify(valueToStore), mergedOptions) - } - } - } catch (error) { - console.warn(`Error setting cookie "${key}":`, error) - } + try { + setStoredValue((prev) => { + const valueToStore = value instanceof Function ? value(prev) : value + + if (typeof document !== "undefined") { + if (valueToStore === null || valueToStore === undefined) { + deleteCookie(key, { path: mergedOptions.path, domain: mergedOptions.domain }) + } else { + setCookie(key, JSON.stringify(valueToStore), mergedOptions) + } + } + + return valueToStore + }) + } catch (error) { + console.warn(`Error setting cookie "${key}":`, error) + } }, - [key, storedValue, mergedOptions] + [key, mergedOptions] )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/use-cookie-storage.tsx` around lines 28 - 46, The setValue callback closes over storedValue causing it to change identity on every state update; change it to use the functional form of setStoredValue: inside useCallback call setStoredValue(prev => { const valueToStore = value instanceof Function ? value(prev) : value; /* update cookie/delete based on valueToStore */ return valueToStore }); then remove storedValue from the useCallback dependency array and keep key and mergedOptions, and continue using deleteCookie/setCookie with mergedOptions and key; ensure cookie updates use the computed valueToStore rather than the closed-over storedValue.src/hooks/use-session-storage.tsx (1)
22-36: ⚡ Quick winConsider using functional setState to stabilize the
setValuecallback.Same issue as in
use-cookie-storage.tsx: includingstoredValuein the dependencies array creates a newsetValuereference on every state change. Using functional setState removes this dependency and ensures successive updates are properly chained through React's state queue.♻️ Refactor to use functional setState
const setValue: Dispatch<SetStateAction<T>> = useCallback( (value) => { - try { - const valueToStore = value instanceof Function ? value(storedValue) : value - setStoredValue(valueToStore) - - if (typeof window !== "undefined") { - window.sessionStorage.setItem(key, JSON.stringify(valueToStore)) - } - } catch (error) { - console.warn(`Error setting sessionStorage key "${key}":`, error) - } + try { + setStoredValue((prev) => { + const valueToStore = value instanceof Function ? value(prev) : value + + if (typeof window !== "undefined") { + window.sessionStorage.setItem(key, JSON.stringify(valueToStore)) + } + + return valueToStore + }) + } catch (error) { + console.warn(`Error setting sessionStorage key "${key}":`, error) + } }, - [key, storedValue] + [key] )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/use-session-storage.tsx` around lines 22 - 36, The setValue callback currently closes over storedValue and lists it in the useCallback deps which recreates setValue on every state change; change setValue (the function passed to useCallback) to use functional setState with setStoredValue(prev => { const valueToStore = typeof value === "function" ? value(prev) : value; /* persist to sessionStorage using key */ return valueToStore; }) so you compute valueToStore from the previous state instead of reading storedValue, then remove storedValue from the dependency array so useCallback only depends on key; keep the sessionStorage write (window.sessionStorage.setItem(key, JSON.stringify(valueToStore))) and the existing try/catch and warning behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/dashboard/`(active)/breadcrumb.tsx:
- Line 69: Remove the debug logging statement that prints route breadcrumbs to
the client: delete the console.log(breadcrumbs) call in
src/app/dashboard/(active)/breadcrumb.tsx (the place where the breadcrumbs
variable is used/rendered) so that user-facing code no longer outputs route
details; if needed for development replace it with a guarded debug logger that's
disabled in production or move logging behind a NODE_ENV check in the Breadcrumb
rendering logic.
---
Nitpick comments:
In `@src/app/dashboard/`(active)/breadcrumb.tsx:
- Around line 63-66: The breadcrumb logic currently uses a hard-coded index
check (i !== 1) when building breadcrumb objects in breadcrumb.tsx; replace this
with route-aware link eligibility by consulting NAV_MAP (or the existing route
metadata) instead of relying on the iteration index. Locate where
breadcrumbs.push({ title, url: i !== 1 ? currentPath : undefined }) is used and
change it to compute a boolean like isLinkable = NAV_MAP[currentPath]?.linkable
|| NAV_MAP[currentPath]?.isPage (or whatever metadata key denotes navigable
pages) and then push { title, url: isLinkable ? currentPath : undefined };
remove dependence on the variable i/current depth so future route structure
changes won’t break breadcrumb link behavior.
In `@src/hooks/use-cookie-storage.tsx`:
- Around line 28-46: The setValue callback closes over storedValue causing it to
change identity on every state update; change it to use the functional form of
setStoredValue: inside useCallback call setStoredValue(prev => { const
valueToStore = value instanceof Function ? value(prev) : value; /* update
cookie/delete based on valueToStore */ return valueToStore }); then remove
storedValue from the useCallback dependency array and keep key and
mergedOptions, and continue using deleteCookie/setCookie with mergedOptions and
key; ensure cookie updates use the computed valueToStore rather than the
closed-over storedValue.
In `@src/hooks/use-session-storage.tsx`:
- Around line 22-36: The setValue callback currently closes over storedValue and
lists it in the useCallback deps which recreates setValue on every state change;
change setValue (the function passed to useCallback) to use functional setState
with setStoredValue(prev => { const valueToStore = typeof value === "function" ?
value(prev) : value; /* persist to sessionStorage using key */ return
valueToStore; }) so you compute valueToStore from the previous state instead of
reading storedValue, then remove storedValue from the dependency array so
useCallback only depends on key; keep the sessionStorage write
(window.sessionStorage.setItem(key, JSON.stringify(valueToStore))) and the
existing try/catch and warning behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 20b31a27-f364-49fd-9b2a-70d335ddc6c2
📒 Files selected for processing (8)
src/app/dashboard/(active)/account/page.tsxsrc/app/dashboard/(active)/azure/page.tsxsrc/app/dashboard/(active)/breadcrumb.tsxsrc/components/dashboard-sidebar/data.tsxsrc/components/dashboard-sidebar/main-nav.tsxsrc/hooks/use-cookie-storage.tsxsrc/hooks/use-session-storage.tsxsrc/index.css
💤 Files with no reviewable changes (1)
- src/app/dashboard/(active)/azure/page.tsx
✅ Files skipped from review due to trivial changes (1)
- src/app/dashboard/(active)/account/page.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/dashboard-sidebar/data.tsx
- src/components/dashboard-sidebar/main-nav.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/app/dashboard/(active)/telegram/grants/page.tsx (1)
7-8: ⚡ Quick winFetch grants in parallel to reduce server latency.
These two calls are independent and can be awaited together.
Suggested change
- const { grants: ongoing } = await trpc.tg.grants.getOngoing.query() - const { grants: scheduled } = await trpc.tg.grants.getScheduled.query() + const [{ grants: ongoing }, { grants: scheduled }] = await Promise.all([ + trpc.tg.grants.getOngoing.query(), + trpc.tg.grants.getScheduled.query(), + ])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/dashboard/`(active)/telegram/grants/page.tsx around lines 7 - 8, The two independent TRPC calls trpc.tg.grants.getOngoing.query() and trpc.tg.grants.getScheduled.query() are awaited sequentially causing extra latency; run them in parallel with Promise.all and destructure the results into { grants: ongoing } and { grants: scheduled } (locate the calls to trpc.tg.grants.getOngoing.query and trpc.tg.grants.getScheduled.query in page.tsx) so both requests execute concurrently and then use the resolved values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/ui/alert.tsx`:
- Line 12: The "info" variant mapping in the Alert component uses a low-contrast
pairing ("bg-primary/20" with "text-primary-foreground"); update the variant
entry named "info" in src/components/ui/alert.tsx so it uses a higher-contrast
text token such as "text-primary" (or the project's designated
foreground-for-tinted-surfaces token) instead of "text-primary-foreground", and
re-run a contrast/accessibility check to confirm the pairing meets WCAG contrast
thresholds.
---
Nitpick comments:
In `@src/app/dashboard/`(active)/telegram/grants/page.tsx:
- Around line 7-8: The two independent TRPC calls
trpc.tg.grants.getOngoing.query() and trpc.tg.grants.getScheduled.query() are
awaited sequentially causing extra latency; run them in parallel with
Promise.all and destructure the results into { grants: ongoing } and { grants:
scheduled } (locate the calls to trpc.tg.grants.getOngoing.query and
trpc.tg.grants.getScheduled.query in page.tsx) so both requests execute
concurrently and then use the resolved values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5e87c6ff-eb34-4f0a-a501-635f2bfea946
📒 Files selected for processing (8)
src/app/dashboard/(active)/breadcrumb.tsxsrc/app/dashboard/(active)/layout.tsxsrc/app/dashboard/(active)/page.tsxsrc/app/dashboard/(active)/telegram/grants/page.tsxsrc/app/dashboard/(active)/telegram/groups/page.tsxsrc/components/dashboard-sidebar/main-nav.tsxsrc/components/ui/alert.tsxsrc/components/ui/button.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/app/dashboard/(active)/layout.tsx
- src/app/dashboard/(active)/breadcrumb.tsx
- src/components/dashboard-sidebar/main-nav.tsx
lorenzocorallo
added
area: frontend
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.