Skip to content

Web sidebar position fixed#1561

Merged
ragnep merged 10 commits intomainfrom
new-layout-responsiveness
Oct 23, 2025
Merged

Web sidebar position fixed#1561
ragnep merged 10 commits intomainfrom
new-layout-responsiveness

Conversation

@ragnep
Copy link
Copy Markdown
Contributor

@ragnep ragnep commented Oct 22, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Improved layout margin calculation to account for scrollbars, ensuring consistent centering and alignment.
    • Reduced unnecessary left offsets for sidebar and submenu when closed while preserving open-state spacing.
  • Style

    • Minor root-container alignment tweak for better spacing and justification.
    • Enabled stable scrollbar behavior for a more consistent scrolling experience.
  • Behavior

    • Footer rendering simplified to show consistently when not hidden.
  • Performance

    • Small rendering optimizations with no public API changes.

Signed-off-by: ragnep <ragneinfo@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 22, 2025

Walkthrough

Memoizes layout CSS variables in WebLayout, adjusts sidebar wrapper and submenu left margins, updates global styles to compensate for scrollbar width and adds scrollbar-gutter: stable both-edges (duplicate insertion present), and simplifies Footer rendering and layout composition in providers. No public API signature changes.

Changes

Cohort / File(s) Summary
Memoized CSS variables
components/layout/WebLayout.tsx
Replaced static cssVars with a useMemo-computed object dependent on sidebarWidth, preserving the same CSS variable keys; added tw-justify-between to the root div className.
Sidebar wrapper margin tweak
components/utils/sidebar/SidebarLayout.tsx
Changed wrapper conditional class from !open ? "tw-ml-2" : "tw-ml-[320px]" to !open ? "tw-ml-0" : "tw-ml-[320px]" (effectively removing the small left margin when closed); no other layout logic changed.
Submenu left-margin removal
components/layout/sidebar/nav/WebSidebarSubmenu.tsx
Removed tw-ml-2 from submenu container classes, reducing left offset; no structural or behavioral changes.
Scrollbar compensation & gutter
styles/globals.scss
Added --scrollbar-compensation: calc(100vw - 100%);, adjusted --layout-margin to subtract that compensation, and inserted html { scrollbar-gutter: stable both-edges; } (appears duplicated in diff).
Footer rendering & layout composition
FooterWrapper.tsx, components/providers/LayoutWrapper.tsx
FooterWrapper.tsx: removed unused imports and conditional footer branches to render <Footer /> directly when not hidden. LayoutWrapper.tsx: renamed shouldUseSmallScreenLayoutisSmallLayout, and changed rendering to wrap LayoutComponent around a fragment containing both children and FooterWrapper.

Sequence Diagram(s)

sequenceDiagram
    participant WL as WebLayout
    participant U as useMemo (cssVars)
    participant B as Browser / DOM

    WL->>B: measure layout (on mount / resize)
    WL->>U: sidebarWidth or measurements change
    U-->>B: apply CSS vars (--sidebar-width, --collapsed-width, --layout-max, --layout-margin)
    Note right of B: CSS updates affect layout rendering
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • analyticsflowee

Poem

🐇 I hopped through vars and memo’d with care,
Nibbled margins tiny, made submenus spare.
I chased the scrollbar, gave gutters room to play,
Wrapped content tidy — footer joins the fray.
Hooray for layouts — carrots for the day! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Web sidebar position fixed" is related to the actual changes in the pull request. The changes across multiple files do involve sidebar positioning adjustments, including margin modifications in SidebarLayout (left margin changed from tw-ml-2 to tw-ml-0), removal of left offset in WebSidebarSubmenu, and layout restructuring. However, the title only partially captures the scope of the work—it omits significant changes such as scrollbar compensation in the global styles, layout margin recalculation, footer wrapper simplification, and layout wrapper restructuring. The title is specific and not vague, but it focuses narrowly on one aspect rather than conveying the full breadth of layout improvements.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch new-layout-responsiveness

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c639d41 and dbb2a8e.

📒 Files selected for processing (1)
  • components/providers/LayoutWrapper.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before props

Use TypeScript across the codebase

Files:

  • components/providers/LayoutWrapper.tsx
**/*.tsx

📄 CodeRabbit inference engine (.cursorrules)

**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for styling

Use React functional components with hooks for UI components

Files:

  • components/providers/LayoutWrapper.tsx
🧬 Code graph analysis (1)
components/providers/LayoutWrapper.tsx (1)
FooterWrapper.tsx (1)
  • FooterWrapper (8-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
components/providers/LayoutWrapper.tsx (2)

70-71: LGTM: Variable rename improves clarity.

The rename from shouldUseSmallScreenLayout to isSmallLayout is clearer and more concise while preserving the same logic.


84-89: Remove fragment wrapper; verify footer positioning across layout variants.

Two valid observations:

  1. The fragment wrapper is redundant. React automatically handles multiple children, so remove it:
  return (
    <LayoutComponent>
-     <>
        {children}
        <FooterWrapper />
-     </>
    </LayoutComponent>
  );
  1. Moving FooterWrapper as a child of LayoutComponent is significant. Verify the footer renders correctly across all three layout variants:
    • WebLayout: Footer now renders inside <main className="layout-main">
    • SmallScreenLayout: Footer now renders inside <main className="tw-transition-opacity tw-duration-300">
    • MobileLayout: Delegates to AppLayout or SmallScreenLayout; verify footer appears correctly in both

Test that footer positioning, scrolling behavior, and styling work as intended in each layout.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 985b31e and 82eac8a.

📒 Files selected for processing (2)
  • components/layout/WebLayout.tsx (2 hunks)
  • components/utils/sidebar/SidebarLayout.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before props

**/*.{ts,tsx}: Use TypeScript for source code and follow existing code style and naming conventions
Adhere to clean code standards as measured by SonarQube

Files:

  • components/utils/sidebar/SidebarLayout.tsx
  • components/layout/WebLayout.tsx
**/*.tsx

📄 CodeRabbit inference engine (.cursorrules)

**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for styling

Implement React components as functional components using hooks (no class components)

Files:

  • components/utils/sidebar/SidebarLayout.tsx
  • components/layout/WebLayout.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
components/utils/sidebar/SidebarLayout.tsx (1)

120-122: LGTM! Simplified margin logic.

Removing the small left margin when the sidebar is closed aligns with the dynamic layout margin system introduced in WebLayout.tsx. The change simplifies the conditional logic while maintaining the 320px offset when the sidebar is open.

components/layout/WebLayout.tsx (5)

3-9: LGTM! Required hooks imported.

The additional React hooks are necessary for the dynamic layout margin calculation and are correctly imported.


36-39: LGTM! Ref and state properly initialized.

The ref and state declarations are correctly typed and initialized for the layout margin calculation logic.


83-95: LGTM! CSS variables properly memoized.

The memoized CSS variables object efficiently handles the dynamic layout margin with correct dependencies and conditional inclusion of the --layout-margin property.


99-99: LGTM! Ref correctly assigned.

The ref attachment to the root div element enables the position measurement logic in the useEffect.


97-107: LGTM! Memoized CSS variables applied efficiently.

The application of the memoized cssVars object to the style prop improves performance by preventing unnecessary object recreation on each render.

Comment thread components/layout/WebLayout.tsx Outdated
Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
components/providers/LayoutWrapper.tsx (2)

70-71: Avoid "use" prefix for non-hook variables.

The variable useSmallLayout is not a React hook but uses the conventional "use" prefix reserved for hooks. This violates React naming conventions and may confuse developers or trigger linter warnings.

Consider renaming to better reflect its nature as a computed boolean:

-  const useSmallLayout =
+  const isSmallLayout =
     hasTouchScreen && (isSmallScreen || isTouchTabletViewport);

Then update line 75:

-  } else if (useSmallLayout) {
+  } else if (isSmallLayout) {
     LayoutComponent = SmallScreenLayout;

79-88: Move content creation after early return check.

The content constant is created unconditionally, including when isAccessOrRestricted is true. This causes FooterWrapper to mount and execute its hooks (useState, useEffect, usePathname) even though the content is never used on access/restricted routes.

Move the content creation after the early return:

-  const content = (
-    <>
-      {children}
-      <FooterWrapper />
-    </>
-  );
-
   if (isAccessOrRestricted) {
     return <>{children}</>;
   }
 
+  const content = (
+    <>
+      {children}
+      <FooterWrapper />
+    </>
+  );
+
   return <LayoutComponent>{content}</LayoutComponent>;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5e37767 and c639d41.

📒 Files selected for processing (2)
  • FooterWrapper.tsx (1 hunks)
  • components/providers/LayoutWrapper.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before props

Use TypeScript across the codebase

Files:

  • components/providers/LayoutWrapper.tsx
  • FooterWrapper.tsx
**/*.tsx

📄 CodeRabbit inference engine (.cursorrules)

**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for styling

Use React functional components with hooks for UI components

Files:

  • components/providers/LayoutWrapper.tsx
  • FooterWrapper.tsx
🧬 Code graph analysis (2)
components/providers/LayoutWrapper.tsx (2)
components/layout/SmallScreenLayout.tsx (1)
  • SmallScreenLayout (23-116)
FooterWrapper.tsx (1)
  • FooterWrapper (8-61)
FooterWrapper.tsx (1)
components/footer/Footer.tsx (1)
  • Footer (3-95)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
FooterWrapper.tsx (1)

60-60: LGTM! Simplified footer rendering.

The removal of sidebar-specific layout logic (sidebarWidth, isMobile, isNarrow) and conditional padding simplifies this component. Footer positioning now relies on parent layout (WebLayout with CSS variables), which is a cleaner separation of concerns.

Signed-off-by: ragnep <ragneinfo@gmail.com>
@ragnep ragnep merged commit e088fac into main Oct 23, 2025
7 checks passed
@ragnep ragnep deleted the new-layout-responsiveness branch October 23, 2025 08:02
@sonarqubecloud
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot mentioned this pull request Nov 3, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Dec 30, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants