Feature/fin 021 parts/sidebar#75
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces a comprehensive sidebar and sheet UI component library with mobile responsiveness support, adds responsive hooks and navigation models, updates the profile layout to conditionally render based on viewport size, wraps the app with a tooltip provider, and reorganizes CSS variable declarations. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (10)
src/client/shared/ui/ui-sidebar/styles/sidebar-inset-styles.scss-16-18 (1)
16-18:⚠️ Potential issue | 🟡 MinorFix collapsed-state selector; current nesting prevents the rule from matching.
On Line 16,
&.peer[data-state='collapsed'] ~ &tries to match.sidebar-inset.peer[data-state='collapsed'], but the.sidebar-insetelement doesn't have thepeerclass (only.sidebar-wrapperdoes). The collapsed left margin rule will never apply.Proposed selector fix
`@media` (min-width: 768px) { .peer[data-variant='inset'] ~ & { margin: 0.5rem; margin-left: 0; border-radius: 0.75rem; box-shadow: 0 1px 2px 0 rgb(0 0 0 / 0.05); - &.peer[data-state='collapsed'] ~ & { - margin-left: 0.5rem; - } } + + .peer[data-variant='inset'][data-state='collapsed'] ~ & { + margin-left: 0.5rem; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/shared/ui/ui-sidebar/styles/sidebar-inset-styles.scss` around lines 16 - 18, The collapsed-state selector is nested incorrectly: replace the failing selector "&.peer[data-state='collapsed'] ~ &" with a selector that targets .sidebar-inset when a sibling .sidebar-wrapper has the peer collapsed state, e.g. ".sidebar-wrapper.peer[data-state='collapsed'] ~ &"; update the rule block containing that selector so the margin-left: 0.5rem applies when the .sidebar-wrapper element has the peer and data-state="collapsed".src/client/shared/ui/ui-sidebar/styles/sidebar-menu-action-styles.scss-13-13 (1)
13-13:⚠️ Potential issue | 🟡 Minor
outline-style: hiddenis not valid CSS and will be dropped by the browser.The
hiddenvalue is not valid for theoutline-styleproperty. Per the CSS spec and MDN, valid values are:auto,none,dotted,dashed,solid,double,groove,ridge,inset,outset. Useoutline: none;instead.Suggested fix
- outline-style: hidden; + outline: none;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/shared/ui/ui-sidebar/styles/sidebar-menu-action-styles.scss` at line 13, Replace the invalid CSS declaration "outline-style: hidden" with a valid rule; change it to "outline: none" (or "outline-style: none") in the same selector where "outline-style: hidden" appears so the browser will not drop the rule; keep any accessibility considerations in mind (consider using :focus-visible later if you need to hide outlines only for mouse users).src/client/shared/ui/ui-sidebar/styles/sidebar-group-action-styles.scss-13-13 (1)
13-13:⚠️ Potential issue | 🟡 Minor
outline-style: hiddenis invalid CSS and will be ignored. Use valid CSS syntax instead.Line 13 uses an invalid value for the
outline-styleproperty ("hidden" is not a valid outline-style keyword). The declaration has no effect and will be ignored by browsers. This pattern appears in 5 sidebar component files. Replace with a valid CSS declaration to remove the outline.Suggested fix
- outline-style: hidden; + outline: none;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/shared/ui/ui-sidebar/styles/sidebar-group-action-styles.scss` at line 13, Replace the invalid CSS declaration "outline-style: hidden" (found in sidebar-group-action-styles.scss and four other sidebar component files) with a valid rule such as "outline: none;" or "outline-style: none;" to remove the outline; update each file where "outline-style: hidden" appears and consider using :focus-visible to restore accessible focus styling where needed.src/client/shared/ui/ui-sidebar/ui-sidebar-menu-action.tsx-16-25 (1)
16-25:⚠️ Potential issue | 🟡 MinorAdd explicit
type="button"to prevent accidental form submissions.When
asChildisfalse, the native<button>renders without an explicittypeattribute, defaulting totype="submit"in form contexts per the HTML specification. This can trigger unintended form submissions.This pattern is already established in similar components like
UiButton,UiIconButton, andUiPurpleButtonwhich all explicitly settype={!asChild ? (type ?? 'button') : undefined}for consistency and safety.Suggested fix
export function UiSidebarMenuAction({ className, asChild = false, showOnHover = false, ...props }: React.ComponentProps<'button'> & { asChild?: boolean; showOnHover?: boolean; }) { const Comp = asChild ? Slot.Root : 'button'; return ( <Comp data-slot="sidebar-menu-action" data-sidebar="menu-action" data-show-on-hover={showOnHover} + type={!asChild ? 'button' : undefined} className={cn('sidebar-menu-action', className)} {...props} /> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/shared/ui/ui-sidebar/ui-sidebar-menu-action.tsx` around lines 16 - 25, The returned element uses Comp = asChild ? Slot.Root : 'button' and can render a native button without an explicit type (causing accidental form submits); modify the component to accept an optional type prop and pass type={!asChild ? (type ?? 'button') : undefined} into <Comp ... /> so when asChild is false the native button defaults to 'button' and when asChild is true the type is undefined (preserving Slot behavior), matching patterns used in UiButton/UiIconButton/UiPurpleButton.src/app/(profile)/layout.tsx-21-26 (1)
21-26:⚠️ Potential issue | 🟡 MinorAvoid first-paint desktop sidebar on mobile devices.
On Line 22 and Line 26, branching depends on
useIsMobile(), but that hook returnsfalseon initial render (src/client/shared/hooks/is-mobile/is-mobile.hook.ts:3-19). Mobile users can briefly see desktop sidebar, then a swap to mobile nav.💡 Suggested fix (gate responsive rendering until mounted)
+'use client'; + +import * as React from 'react'; import { ChildrenComponentProps } from '@frontend/shared/models/component-with-chilren.model'; import { useUserGuard } from '@frontend/entities/profile/auth-guard.hook'; import { AuthorizedUserProvider } from '@frontend/shared/services/user-information/authorized-user.hook'; import { ProfileSidebar } from '@frontend/widgets/profile-sidebar/profile-sidebar'; import { useIsMobile } from '@frontend/shared/hooks/is-mobile/is-mobile.hook'; import { UserMobileNavigationBar } from '@frontend/widgets/mobile-navigation/navigation-bar/navigation-bar'; import { cn } from '@frontend/shared/utils/cn.util'; export default function UserLayoutPage({ children }: ChildrenComponentProps) { const user = useUserGuard(); const isMobile = useIsMobile(); + const [mounted, setMounted] = React.useState(false); + + React.useEffect(() => { + setMounted(true); + }, []); if (!user) { return null; } return ( <AuthorizedUserProvider> - <div className={cn('size-full flex', isMobile && 'flex-col')}> - {!isMobile && <ProfileSidebar />} + <div className={cn('size-full flex', mounted && isMobile && 'flex-col')}> + {mounted && !isMobile && <ProfileSidebar />} <div className="flex-1">{children}</div> - {isMobile && <UserMobileNavigationBar />} + {mounted && isMobile && <UserMobileNavigationBar />} </div> </AuthorizedUserProvider> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(profile)/layout.tsx around lines 21 - 26, The layout renders ProfileSidebar or UserMobileNavigationBar based directly on useIsMobile(), which returns false on first render and causes a desktop sidebar flash on mobile; wrap the responsive branching in a "mounted" guard so you only decide between ProfileSidebar and UserMobileNavigationBar after the component is mounted (e.g., add a local hasMounted state set true in useEffect or reuse an existing useIsMounted hook) and in layout.tsx defer rendering those two components until hasMounted is true to prevent the initial incorrect desktop render.src/client/shared/ui/ui-sidebar/styles/sidebar-group-label-styles.scss-13-13 (1)
13-13:⚠️ Potential issue | 🟡 MinorReplace invalid
outline-style: hiddenwith valid CSS.
outline-style: hiddenis not a valid CSS value and will be ignored by browsers. Thehiddenkeyword is valid only forborder-style, notoutline-style.Fix
- outline-style: hidden; + outline: none;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/shared/ui/ui-sidebar/styles/sidebar-group-label-styles.scss` at line 13, Replace the invalid declaration "outline-style: hidden" with a valid outline reset such as "outline: none;" (or "outline-style: none;") in the same rule so the browser will honor it; locate the rule containing the "outline-style: hidden" declaration and update that line to "outline: none;" to correctly remove the outline.src/client/shared/ui/ui-sidebar/styles/sidebar-rail-styles.scss-35-41 (1)
35-41:⚠️ Potential issue | 🟡 MinorLikely selector typo prevents cursor styles from applying.
Line 35 and Line 39 use
.in[...], while surrounding logic uses.group[...]. This likely breaks the intended resize cursor behavior.💡 Proposed fix
- .in[data-side='left'] & { + .group[data-side='left'] & { cursor: w-resize; } - .in[data-side='right'] & { + .group[data-side='right'] & { cursor: e-resize; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/shared/ui/ui-sidebar/styles/sidebar-rail-styles.scss` around lines 35 - 41, The selectors `.in[data-side='left'] &` and `.in[data-side='right'] &` are likely typos and prevent the resize cursors from applying; change those selectors to `.group[data-side='left'] &` and `.group[data-side='right'] &` respectively so they match the surrounding logic and enable the intended w-resize/e-resize cursor behavior (update the two selector lines referencing `.in` to `.group`).src/client/shared/ui/ui-sidebar/ui-sidebar.tsx-59-61 (1)
59-61:⚠️ Potential issue | 🟡 MinorFix the SR-only Ukrainian copy.
Line 61 has a typo:
Відбражається→Відображається.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/shared/ui/ui-sidebar/ui-sidebar.tsx` around lines 59 - 61, The Ukrainian SR-only copy in the UiSheetHeader is misspelled; update the UiSheetDescription text inside the UiSheetHeader (where UiSheetHeader, UiSheetTitle and UiSheetDescription are defined) to fix the typo by changing "Відбражається лише на мобільних пристроях" to "Відображається лише на мобільних пристроях".src/client/shared/ui/ui-sidebar/ui-sidebar-menu-button.tsx-34-43 (1)
34-43:⚠️ Potential issue | 🟡 MinorDefault the native button to
type="button".Without an explicit type, this submits the nearest form. Sidebar navigation clicks should not trigger form submission.
💡 Suggested fix
export function UiSidebarMenuButton({ asChild = false, isActive = false, variant = 'default', size = 'default', tooltip, + type, className, ...props }: UiSidebarMenuButtonProps) { @@ <Comp data-slot="sidebar-menu-button" data-sidebar="menu-button" data-size={size} data-active={isActive} data-variant={variant} className={cn('sidebar-menu-button', className)} + {...(!asChild ? { type: type ?? 'button' } : {})} {...props} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/shared/ui/ui-sidebar/ui-sidebar-menu-button.tsx` around lines 34 - 43, The rendered menu button (variable "button" using "Comp" in ui-sidebar-menu-button.tsx) lacks an explicit type, so clicks inside a form can trigger submission; ensure a native button gets type="button" by passing type="button" to the JSX element (place type="button" before {...props}) or set a default in the component props (e.g., default type = 'button' when destructuring) and, if "Comp" can be other elements, only apply this default when Comp === 'button'.src/client/shared/ui/ui-sidebar/ui-sidebar-provider.tsx-17-25 (1)
17-25:⚠️ Potential issue | 🟡 MinorMake the context setters real React state setters, or keep them boolean-only everywhere.
SidebarContextPropsexposessetOpen/setOpenMobileas(open: boolean) => void, but the provider already uses updater functions intoggleSidebar().setOpenis wrapped to accept both forms, andsetOpenMobileis a direct React state setter that also supports updater functions. This means the public API is typed narrower than the implementation, and callers would violate the declared type contract when passing updater functions.💡 Suggested direction
export type SidebarContextProps = { state: 'expanded' | 'collapsed'; open: boolean; - setOpen: (open: boolean) => void; + setOpen: React.Dispatch<React.SetStateAction<boolean>>; openMobile: boolean; - setOpenMobile: (open: boolean) => void; + setOpenMobile: React.Dispatch<React.SetStateAction<boolean>>; isMobile: boolean; toggleSidebar: () => void; };If you keep updater-form support, resolve the next value from the previous state inside the setter path instead of evaluating it against the closed-over
open.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/shared/ui/ui-sidebar/ui-sidebar-provider.tsx` around lines 17 - 25, SidebarContextProps currently types setOpen and setOpenMobile as (open: boolean) => void while the provider (see toggleSidebar and the provider's internal setters) accepts React-style updater functions; update the public types to accept React.Dispatch<React.SetStateAction<boolean>> (or a union type like boolean | ((prev:boolean)=>boolean)) so callers can pass updater functions, and if keeping updater support ensure any custom setter (e.g., the wrapper around setOpen used by toggleSidebar) resolves the next value from the previous state (invoke functional updater when a function is passed) instead of relying on closed-over open; update the type for setOpen and setOpenMobile and adjust the provider wrapper logic to handle functional updaters accordingly.
🧹 Nitpick comments (4)
src/app/client-layout.tsx (1)
25-28: Consolidate tooltip provider ownership to avoid nested contexts.
MainLayoutnow provides tooltip context globally, whileUiSidebarProvideralso wraps its subtree with a tooltip provider (src/client/shared/ui/ui-sidebar/ui-sidebar-provider.tsx, around Line 110-Line 125). Consider keeping a single owner to avoid config drift and unnecessary nesting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/client-layout.tsx` around lines 25 - 28, MainLayout currently wraps children with UiTooltipProvider (UiTooltipProvider in src/app/client-layout.tsx) while UiSidebarProvider also creates a tooltip context (ui-sidebar-provider.tsx around the provider block), causing nested tooltip contexts; pick a single owner—prefer the global MainLayout—and remove the UiTooltipProvider wrapper from UiSidebarProvider (delete the <UiTooltipProvider> ... </UiTooltipProvider> block in UiSidebarProvider and rely on the existing UiTooltipProvider in client-layout.tsx), ensuring consumers still import/use UiTooltip hooks/components unchanged.src/client/shared/ui/ui-sheet/styles/ui-sheet-overlay-styles.scss (1)
7-24: Add reduced-motion fallback for overlay animations.Current open/close animations run unconditionally; consider respecting
prefers-reduced-motionfor accessibility.♿ Suggested SCSS addition
.sheet-overlay { position: fixed; inset: 0; z-index: 50; background-color: rgba(0, 0, 0, 0.5); &[data-state='open'] { animation: fadeIn 500ms cubic-bezier(0.4, 0, 0.2, 1) both; } &[data-state='closed'] { animation: fadeOut 300ms cubic-bezier(0.4, 0, 0.2, 1) both; } } + +@media (prefers-reduced-motion: reduce) { + .sheet-overlay[data-state='open'], + .sheet-overlay[data-state='closed'] { + animation: none; + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/shared/ui/ui-sheet/styles/ui-sheet-overlay-styles.scss` around lines 7 - 24, Respect users' prefers-reduced-motion by wrapping the open/close animation rules for &[data-state='open'] and &[data-state='closed'] (which currently use the fadeIn/fadeOut keyframes) in a `@media` (prefers-reduced-motion: reduce) query and set animation: none (or a short non-animated fallback, e.g., animation: none !important; opacity: 1 for open, opacity: 0 for closed) so the fadeIn/fadeOut keyframes are not played for reduced-motion users.src/client/shared/hooks/is-mobile/is-mobile.hook.ts (1)
10-15: Usemql.matchesdirectly to avoid duplicated breakpoint logic.Line 11 and Line 14 can read from the media query object itself, which keeps state updates tied to the actual query condition.
Refactor suggestion
React.useEffect(() => { const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`); const onChange = () => { - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT); + setIsMobile(mql.matches); }; mql.addEventListener('change', onChange); - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT); + setIsMobile(mql.matches); return () => mql.removeEventListener('change', onChange); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/shared/hooks/is-mobile/is-mobile.hook.ts` around lines 10 - 15, The hook duplicates breakpoint logic by computing window.innerWidth < MOBILE_BREAKPOINT in both the onChange handler and initial setIsMobile; instead read the media query state directly from the MediaQueryList (mql.matches) to keep updates consistent with the query. Update the onChange callback (onChange) to call setIsMobile(mql.matches) and initialize state with setIsMobile(mql.matches) instead of recalculating via MOBILE_BREAKPOINT, leaving mql.addEventListener('change', onChange) and cleanup via mql.removeEventListener('change', onChange) unchanged.src/client/shared/ui/ui-sidebar/ui-sidebar-rail.tsx (1)
18-22: Preserve built-in toggle behavior when customonClickis passed.Because
{...props}is spread afteronClick, a consumer-providedonClickcan replacetoggleSidebar.♻️ Suggested refactor
-export function UiSidebarRail({ className, ...props }: React.ComponentProps<'button'>) { +export function UiSidebarRail({ className, onClick, ...props }: React.ComponentProps<'button'>) { const { toggleSidebar } = useSidebar(); return ( <button @@ - onClick={toggleSidebar} + onClick={(event) => { + onClick?.(event); + if (!event.defaultPrevented) toggleSidebar(); + }} title="Toggle Sidebar" className={cn('sidebar-rail', className)} {...props} /> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/shared/ui/ui-sidebar/ui-sidebar-rail.tsx` around lines 18 - 22, The current JSX spreads {...props} after setting onClick={toggleSidebar}, allowing a consumer onClick to override toggleSidebar; modify the component so the final click handler composes both: call toggleSidebar and then call props.onClick (if present) rather than letting props replace it — update the element that currently sets onClick={toggleSidebar} to compute a combined handler (using toggleSidebar and props.onClick) or spread props first and set onClick last so toggleSidebar always runs; refer to toggleSidebar, props.onClick, and the element rendering the sidebar-rail to locate where to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/shared/hooks/get-active-route/get-active-route.hook.ts`:
- Around line 39-41: The current prefix check uses pathname.startsWith(path) and
then assigns currentRoute = key during iteration, which causes false positives
(e.g., "/profile" matching "/profiled") and non-deterministic overrides; update
the match to be segment-safe by requiring either pathname === path or
pathname.startsWith(path + "/") (so the path boundary is respected), and make
the selection deterministic by picking the first matching key instead of
overwriting (e.g., use Array.prototype.find on paths or break after setting
currentRoute). Apply this change where currentRoute, paths, pathname, and key
are used so the active route resolution is boundary-aware and
order-deterministic.
In `@src/client/shared/ui/ui-icon-button/props/icon-button.props.ts`:
- Line 6: IconButtonProps currently makes size optional but UiIconButton
forwards size to UiSvgIcon where size is required; fix by either (A) making size
required on IconButtonProps (update the IconButtonProps type to remove the ?
from size) so callers must supply it, or (B) enforce a default inside
UiIconButton (set a sensible default like "medium" in the UiIconButton component
before forwarding to UiSvgIcon) and ensure UiIconButton always passes a defined
size; update references to IconButtonProps, UiIconButton, and the forwarded size
prop accordingly.
In `@src/client/shared/ui/ui-sidebar/styles/sidebar-styles.scss`:
- Around line 35-46: The selectors assume two nested .group ancestors but
UiSidebar (src/client/shared/ui/ui-sidebar/ui-sidebar.tsx) sets data-variant and
data-collapsible on the same wrapper, so the current nested
`.group[data-variant='...'] .group[data-collapsible='icon'] &` never matches;
update the SCSS to combine attributes on the same .group (e.g.,
`.group[data-variant='floating'][data-collapsible='icon'] &` and a corresponding
combined selector for non-floating/non-inset) so the `.sidebar-gap` width rules
apply when both attributes exist on the same element.
In `@src/client/shared/ui/ui-sidebar/ui-sidebar-group-action.tsx`:
- Around line 15-20: The sidebar action component renders a native button when
asChild is false but doesn't set a button type, causing unintended form
submissions; update the component (the render of Comp in
ui-sidebar-group-action.tsx, and apply the same fix to UiSidebarMenuAction) to
ensure when asChild is false you pass an explicit type='button' prop (while
preserving any user-provided type in {...props}), so the default behavior is
safe.
In `@src/client/shared/ui/ui-sidebar/ui-sidebar-header-icon.tsx`:
- Around line 1-3: This file uses React hooks (useState) and the custom hook
useSidebar inside the UiSidebarHeaderIcon component, so add the Next.js client
directive by inserting 'use client' as the very first line of src code (before
any imports) to mark the module as a client component; ensure the string literal
appears exactly as 'use client' on its own line above the existing imports
(affecting UiSidebarHeaderIcon, useState, and useSidebar usage).
In `@src/client/shared/ui/ui-sidebar/ui-sidebar-header.tsx`:
- Line 6: The component UiSidebarHeader currently destructures an unused prop
justifyBetween from React.ComponentProps<'div'> which causes a TS2339 error;
remove justifyBetween from the parameter list (i.e., stop destructuring it) so
the function signature is just ({ className, ...props }:
React.ComponentProps<'div'>) or, if justifyBetween is actually needed, add it to
a custom prop type/interface and use that type instead; ensure any call sites
that pass justifyBetween are updated or the prop is implemented where
appropriate.
In `@src/client/shared/ui/ui-sidebar/ui-sidebar-provider.tsx`:
- Around line 79-89: The global keydown handler in the useEffect (handleKeyDown
/ SIDEBAR_KEYBOARD_SHORTCUT / toggleSidebar) currently fires even when focus is
inside editable controls; modify handleKeyDown to early-return unless the event
target is not an editable element: check that event.target is an HTMLElement and
skip handling when tagName is INPUT, TEXTAREA, SELECT or when
element.isContentEditable is true (and optionally when event.defaultPrevented is
true); only call event.preventDefault() and toggleSidebar() when none of those
editable conditions apply.
In `@src/client/shared/ui/ui-sidebar/ui-sidebar-rail.tsx`:
- Line 17: The close/primary control in UI component UiSidebarRail (the element
with tabIndex={-1}) is currently removed from keyboard navigation; update the
element (in ui-sidebar-rail.tsx) to restore keyboard accessibility by removing
tabIndex or setting it to 0 (or otherwise ensuring it receives focus and handles
keyboard events), and verify the element supports Enter/Space activation and
appropriate ARIA attributes so it remains operable via keyboard.
In `@src/client/shared/ui/ui-sidebar/ui-sidebar.tsx`:
- Around line 42-58: The mobile branch currently passes {...props} to UiSheet
(Dialog.Root) and ignores the container's className and DOM props; update the
mobile implementation so UiSheetContent (the actual mobile DOM node) also
receives the forwarded DOM props and merged className: forward relevant props
(e.g., id, data-*, aria-*, data-testid) and merge the incoming className with
the existing classes on UiSheetContent (while keeping the current inline style
and data attributes) instead of only spreading props onto UiSheet; ensure this
change touches the UiSheet / UiSheetContent usage around openMobile,
setOpenMobile, props and className so mobile gets the same attributes as
desktop.
In `@src/client/shared/ui/ui-tooltip/styles/ui-tooltip-content-styles.scss`:
- Around line 14-43: The side selectors currently override the state animations
because they also set animation-name; remove animation-name from the standalone
&[data-side='...'] rules and instead apply side-specific animation-names only
when the tooltip is opening by using combined selectors with the open states
(e.g. combine &[data-state='delayed-open'] and &[data-state='instant-open'] with
&[data-side='top'|'right'|'bottom'|'left'] to set
slideDownAndFade/slideLeftAndFade/slideUpAndFade/slideRightAndFade), while
keeping &[data-state='closed'] with animation-name: zoomOut and the existing
open state selectors that set zoomIn/zoomOut unchanged.
In `@src/client/widgets/profile-sidebar/profile-sidebar.tsx`:
- Around line 58-59: In ProfileSidebar (profile-sidebar.tsx) the Next.js Image
component is being given string dimensions ("31", "36"); change the width and
height props to numeric values (e.g., width={31} and height={36}) so they are
numbers rather than strings; search for other Image usages in this component
(and any JSX inside ProfileSidebar) and convert any quoted numeric width/height
props to numeric literals as well.
---
Minor comments:
In `@src/app/`(profile)/layout.tsx:
- Around line 21-26: The layout renders ProfileSidebar or
UserMobileNavigationBar based directly on useIsMobile(), which returns false on
first render and causes a desktop sidebar flash on mobile; wrap the responsive
branching in a "mounted" guard so you only decide between ProfileSidebar and
UserMobileNavigationBar after the component is mounted (e.g., add a local
hasMounted state set true in useEffect or reuse an existing useIsMounted hook)
and in layout.tsx defer rendering those two components until hasMounted is true
to prevent the initial incorrect desktop render.
In `@src/client/shared/ui/ui-sidebar/styles/sidebar-group-action-styles.scss`:
- Line 13: Replace the invalid CSS declaration "outline-style: hidden" (found in
sidebar-group-action-styles.scss and four other sidebar component files) with a
valid rule such as "outline: none;" or "outline-style: none;" to remove the
outline; update each file where "outline-style: hidden" appears and consider
using :focus-visible to restore accessible focus styling where needed.
In `@src/client/shared/ui/ui-sidebar/styles/sidebar-group-label-styles.scss`:
- Line 13: Replace the invalid declaration "outline-style: hidden" with a valid
outline reset such as "outline: none;" (or "outline-style: none;") in the same
rule so the browser will honor it; locate the rule containing the
"outline-style: hidden" declaration and update that line to "outline: none;" to
correctly remove the outline.
In `@src/client/shared/ui/ui-sidebar/styles/sidebar-inset-styles.scss`:
- Around line 16-18: The collapsed-state selector is nested incorrectly: replace
the failing selector "&.peer[data-state='collapsed'] ~ &" with a selector that
targets .sidebar-inset when a sibling .sidebar-wrapper has the peer collapsed
state, e.g. ".sidebar-wrapper.peer[data-state='collapsed'] ~ &"; update the rule
block containing that selector so the margin-left: 0.5rem applies when the
.sidebar-wrapper element has the peer and data-state="collapsed".
In `@src/client/shared/ui/ui-sidebar/styles/sidebar-menu-action-styles.scss`:
- Line 13: Replace the invalid CSS declaration "outline-style: hidden" with a
valid rule; change it to "outline: none" (or "outline-style: none") in the same
selector where "outline-style: hidden" appears so the browser will not drop the
rule; keep any accessibility considerations in mind (consider using
:focus-visible later if you need to hide outlines only for mouse users).
In `@src/client/shared/ui/ui-sidebar/styles/sidebar-rail-styles.scss`:
- Around line 35-41: The selectors `.in[data-side='left'] &` and
`.in[data-side='right'] &` are likely typos and prevent the resize cursors from
applying; change those selectors to `.group[data-side='left'] &` and
`.group[data-side='right'] &` respectively so they match the surrounding logic
and enable the intended w-resize/e-resize cursor behavior (update the two
selector lines referencing `.in` to `.group`).
In `@src/client/shared/ui/ui-sidebar/ui-sidebar-menu-action.tsx`:
- Around line 16-25: The returned element uses Comp = asChild ? Slot.Root :
'button' and can render a native button without an explicit type (causing
accidental form submits); modify the component to accept an optional type prop
and pass type={!asChild ? (type ?? 'button') : undefined} into <Comp ... /> so
when asChild is false the native button defaults to 'button' and when asChild is
true the type is undefined (preserving Slot behavior), matching patterns used in
UiButton/UiIconButton/UiPurpleButton.
In `@src/client/shared/ui/ui-sidebar/ui-sidebar-menu-button.tsx`:
- Around line 34-43: The rendered menu button (variable "button" using "Comp" in
ui-sidebar-menu-button.tsx) lacks an explicit type, so clicks inside a form can
trigger submission; ensure a native button gets type="button" by passing
type="button" to the JSX element (place type="button" before {...props}) or set
a default in the component props (e.g., default type = 'button' when
destructuring) and, if "Comp" can be other elements, only apply this default
when Comp === 'button'.
In `@src/client/shared/ui/ui-sidebar/ui-sidebar-provider.tsx`:
- Around line 17-25: SidebarContextProps currently types setOpen and
setOpenMobile as (open: boolean) => void while the provider (see toggleSidebar
and the provider's internal setters) accepts React-style updater functions;
update the public types to accept React.Dispatch<React.SetStateAction<boolean>>
(or a union type like boolean | ((prev:boolean)=>boolean)) so callers can pass
updater functions, and if keeping updater support ensure any custom setter
(e.g., the wrapper around setOpen used by toggleSidebar) resolves the next value
from the previous state (invoke functional updater when a function is passed)
instead of relying on closed-over open; update the type for setOpen and
setOpenMobile and adjust the provider wrapper logic to handle functional
updaters accordingly.
In `@src/client/shared/ui/ui-sidebar/ui-sidebar.tsx`:
- Around line 59-61: The Ukrainian SR-only copy in the UiSheetHeader is
misspelled; update the UiSheetDescription text inside the UiSheetHeader (where
UiSheetHeader, UiSheetTitle and UiSheetDescription are defined) to fix the typo
by changing "Відбражається лише на мобільних пристроях" to "Відображається лише
на мобільних пристроях".
---
Nitpick comments:
In `@src/app/client-layout.tsx`:
- Around line 25-28: MainLayout currently wraps children with UiTooltipProvider
(UiTooltipProvider in src/app/client-layout.tsx) while UiSidebarProvider also
creates a tooltip context (ui-sidebar-provider.tsx around the provider block),
causing nested tooltip contexts; pick a single owner—prefer the global
MainLayout—and remove the UiTooltipProvider wrapper from UiSidebarProvider
(delete the <UiTooltipProvider> ... </UiTooltipProvider> block in
UiSidebarProvider and rely on the existing UiTooltipProvider in
client-layout.tsx), ensuring consumers still import/use UiTooltip
hooks/components unchanged.
In `@src/client/shared/hooks/is-mobile/is-mobile.hook.ts`:
- Around line 10-15: The hook duplicates breakpoint logic by computing
window.innerWidth < MOBILE_BREAKPOINT in both the onChange handler and initial
setIsMobile; instead read the media query state directly from the MediaQueryList
(mql.matches) to keep updates consistent with the query. Update the onChange
callback (onChange) to call setIsMobile(mql.matches) and initialize state with
setIsMobile(mql.matches) instead of recalculating via MOBILE_BREAKPOINT, leaving
mql.addEventListener('change', onChange) and cleanup via
mql.removeEventListener('change', onChange) unchanged.
In `@src/client/shared/ui/ui-sheet/styles/ui-sheet-overlay-styles.scss`:
- Around line 7-24: Respect users' prefers-reduced-motion by wrapping the
open/close animation rules for &[data-state='open'] and &[data-state='closed']
(which currently use the fadeIn/fadeOut keyframes) in a `@media`
(prefers-reduced-motion: reduce) query and set animation: none (or a short
non-animated fallback, e.g., animation: none !important; opacity: 1 for open,
opacity: 0 for closed) so the fadeIn/fadeOut keyframes are not played for
reduced-motion users.
In `@src/client/shared/ui/ui-sidebar/ui-sidebar-rail.tsx`:
- Around line 18-22: The current JSX spreads {...props} after setting
onClick={toggleSidebar}, allowing a consumer onClick to override toggleSidebar;
modify the component so the final click handler composes both: call
toggleSidebar and then call props.onClick (if present) rather than letting props
replace it — update the element that currently sets onClick={toggleSidebar} to
compute a combined handler (using toggleSidebar and props.onClick) or spread
props first and set onClick last so toggleSidebar always runs; refer to
toggleSidebar, props.onClick, and the element rendering the sidebar-rail to
locate where to change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 381af00f-ce95-4917-a3dc-9c32b9b09774
📒 Files selected for processing (79)
src/app/(profile)/layout.tsxsrc/app/(profile)/main/layout.tsxsrc/app/(profile)/profile/budget/regular-operations/page.tsxsrc/app/(profile)/profile/settings/page.tsxsrc/app/(profile)/settings/layout.tsxsrc/app/client-layout.tsxsrc/client/shared/components/sidebar-content/fin-sidebar-content.tsxsrc/client/shared/components/sidebar-content/props/sidebar-content.props.tssrc/client/shared/hooks/get-active-route/get-active-route.hook.tssrc/client/shared/hooks/is-mobile/is-mobile.hook.tssrc/client/shared/models/nav-item.model.tssrc/client/shared/styles/theme.scsssrc/client/shared/ui/ui-icon-button/props/icon-button.props.tssrc/client/shared/ui/ui-sheet/styles/ui-sheet-content-styles.scsssrc/client/shared/ui/ui-sheet/styles/ui-sheet-overlay-styles.scsssrc/client/shared/ui/ui-sheet/ui-sheet-close.tsxsrc/client/shared/ui/ui-sheet/ui-sheet-content.tsxsrc/client/shared/ui/ui-sheet/ui-sheet-description.tsxsrc/client/shared/ui/ui-sheet/ui-sheet-footer.tsxsrc/client/shared/ui/ui-sheet/ui-sheet-header.tsxsrc/client/shared/ui/ui-sheet/ui-sheet-overlay.tsxsrc/client/shared/ui/ui-sheet/ui-sheet-portal.tsxsrc/client/shared/ui/ui-sheet/ui-sheet-title.tsxsrc/client/shared/ui/ui-sheet/ui-sheet-trigger.tsxsrc/client/shared/ui/ui-sheet/ui-sheet.tsxsrc/client/shared/ui/ui-sidebar/styles/sidebar-content-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-footer-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-group-action-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-group-content-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-group-label-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-group-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-header-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-inset-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-menu-action-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-menu-badge-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-menu-button-variant-mixin.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-menu-button-variants.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-menu-item-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-menu-skeleton-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-menu-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-menu-sub-button-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-menu-sub-item-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-menu-sub-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-provider-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-rail-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-separator-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-styles.scsssrc/client/shared/ui/ui-sidebar/styles/sidebar-trigger-styles.scsssrc/client/shared/ui/ui-sidebar/ui-sidebar-content.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-footer.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-group-action.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-group-content.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-group-label.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-group.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-header-icon.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-header-title.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-header.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-inset.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-menu-action.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-menu-badge.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-menu-button.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-menu-item.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-menu-skeleton.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-menu-sub-button.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-menu-sub-item.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-menu-sub.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-menu.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-provider.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-rail.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-separator.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar-trigger.tsxsrc/client/shared/ui/ui-sidebar/ui-sidebar.tsxsrc/client/shared/ui/ui-skeleton/ui-skeleton.tsxsrc/client/shared/ui/ui-tooltip/styles/ui-tooltip-content-styles.scsssrc/client/shared/ui/ui-tooltip/ui-tooltip-content.tsxsrc/client/shared/ui/ui-tooltip/ui-tooltip-provider.tsxsrc/client/shared/ui/ui-tooltip/ui-tooltip-trigger.tsxsrc/client/shared/ui/ui-tooltip/ui-tooltip.tsxsrc/client/widgets/profile-sidebar/profile-sidebar.tsx
💤 Files with no reviewable changes (2)
- src/app/(profile)/main/layout.tsx
- src/app/(profile)/settings/layout.tsx
Title
Add sidebar
Type
Description
Added sidebar, is mobile and get active route hooks
Media
Check list
Summary by CodeRabbit
New Features
Improvements