ECHOES-1308 Remove the thin collapsed sidebar and implement the new undocked interaction model

[david-cho-lerat-sonarsource]

---

Summary by Gitar

• Sidebar interaction model:
  • Removed the thin collapsed sidebar state and implemented a new undocked interaction model with reveal animations.
  • Added SIDEBAR_INTERACTION_ZONE_ATTRIBUTE and pointer-handoff surfaces in GlobalNavigationPrimary and SidebarNavigation to support transient sidebar states without flickering.
• State management:
  • Introduced useLayoutSidebarState to centralize sidebar logic, including media query handling, interaction-zone blur/hover events, and open/docked state synchronization.
• Component API improvements:
  • Forwarded ref and className props to Layout, GlobalNavigationPrimary, SidebarNavigation, SidebarNavigationGroup, and SidebarNavigationFooterPromotionCard for improved consumer control.
• Testing:
  • Added comprehensive integration tests covering keyboard interaction, focus management, and pointer-event handoffs across the new interaction surfaces.
• Storybook updates:
  • Refactored SidebarNavigation stories to demonstrate Docked, UndockedHidden, and UndockedOpen states using a unified SidebarNavigationStoryLayout.

_{This will update automatically on new commits.}

[netlify]

✅ Deploy Preview for echoes-react ready!

Name	Link
🔨 Latest commit	9296e73
🔍 Latest deploy log	https://app.netlify.com/projects/echoes-react/deploys/6a4cf9386fcc3e0008f7faf3
😎 Deploy Preview	https://deploy-preview-716--echoes-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[hashicorp-vault-sonar-prod]

ECHOES-1308

[Copilot]

Pull request overview

This PR updates the Layout sidebar to remove the “thin collapsed” mode and introduce a new undocked interaction model where the sidebar is fully hidden when undocked and only revealed via the top-left trigger (hover/focus/click) or focusing into the sidebar interaction boundary.

Changes:

• Introduces a sidebar “interaction boundary” + safe-area logic to keep the undocked sidebar open while focus/pointer remains in the intended region, and closes it when leaving.
• Refactors SidebarNavigation rendering/styling to use a true hidden/visible open state (opacity/transform/visibility) instead of hover/focus-within collapse behavior.
• Updates Storybook stories, i18n keys, and several tests to match the new docking/undocking and open/close behaviors.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
stories/layout/sidebar-navigation/SidebarNavigationAccordionItem-stories.tsx	Removes stories/examples tied to the old “collapsed/docked icon hiding” behavior.
stories/layout/sidebar-navigation/SidebarNavigation-stories.tsx	Splits the sidebar story into Docked / Undocked Hidden / Undocked Open variants and adds open-on-mount behavior.
stories/layout/Layout-stories.tsx	Makes sidebar accordion item examples compatible with the new undocked model; stabilizes ColorBox rendering deterministically.
src/components/layout/sidebar-navigation/SidebarNavigationHeader.tsx	Removes collapsed-mode-only padding adjustments.
src/components/layout/sidebar-navigation/SidebarNavigationGroup.tsx	Removes collapsed-mode-only label hiding/divider behavior.
src/components/layout/sidebar-navigation/SidebarNavigationFooterPromotionCard.tsx	Removes collapsed-mode-only opacity/transition behavior.
src/components/layout/sidebar-navigation/SidebarNavigationBody.tsx	Removes collapsed-mode scroll/shadow styling and factors spacing tokens into constants.
src/components/layout/sidebar-navigation/SidebarNavigationBaseItem.tsx	Removes accordion “visibility/outline” CSS variable usage and updates icon-hide logic to depend on data-sidebar-open.
src/components/layout/sidebar-navigation/SidebarNavigationAccordionItem.tsx	Simplifies accordion child CSS variables to only control display (open/closed).
src/components/layout/sidebar-navigation/SidebarNavigation.tsx	Implements the new hidden/visible undocked sidebar presentation and wires interaction boundary event handlers + safe area ref.
src/components/layout/sidebar-navigation/tests/SidebarNavigationItem-test.tsx	Updates CSS-variable expectations to match the new accordion integration contract.
src/components/layout/sidebar-navigation/tests/SidebarNavigationAccordionChildItem-test.tsx	Updates CSS-variable expectations to match the new accordion integration contract.
src/components/layout/sidebar-navigation/tests/SidebarNavigation-test.tsx	Updates tests to provide the expanded LayoutContext shape required by the new interaction model.
src/components/layout/LayoutSidebarInteraction.ts	Adds helpers for detecting interaction boundary targets and pointer-safe-area checks.
src/components/layout/LayoutContext.ts	Extends LayoutContext with sidebar open/close + boundary interaction handlers and state.
src/components/layout/Layout.tsx	Implements the new undocked open state and document-level closing behavior; exposes data-sidebar-open.
src/components/layout/global-navigation/GlobalNavigationPrimary.tsx	Updates the trigger button behavior/ARIA to support undocked reveal and non-dockable “open sidebar” behavior.
src/components/layout/global-navigation/tests/GlobalNavigationPrimary-test.tsx	Updates tests for the new trigger rendering and “Open sidebar” behavior.
src/components/layout/tests/Layout-test.tsx	Adds tests for the new undocked interaction model (hover/focus/click open/close, safe-area behavior).
i18n/keys.json	Adds the global_navigation.sidebar.open key for the non-dockable trigger label.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gitar-bot]

Code Review ✅ Approved 2 resolved / 2 findings

Refactors the sidebar to an undocked interaction model and removes the thin collapsed state. Resolved previous findings regarding null guards and test helper usage.

✅ 2 resolved

✅ Quality: Raw truthiness/null guards instead of isDefined() helper

📄 src/components/layout/LayoutSidebarInteraction.ts:23-34 📄 src/components/layout/sidebar-navigation/SidebarNavigation.tsx:72-83
The repo convention requires using the isDefined() helper from ~common/helpers/types for all != null / !== undefined / conditional-render guards (the helper is already used across sibling files like SidebarNavigationBaseItem.tsx, LayoutSlots.tsx, HeaderBase.tsx). Several new code paths use raw checks instead:

• LayoutSidebarInteraction.ts:26 — target.closest(...) !== null (raw null check).
• LayoutSidebarInteraction.ts:34 — if (!sidebarInteractionSafeAreaElement) (raw truthiness guard on an HTMLElement | null).
• SidebarNavigation.tsx:78 — else if (ref !== null) (raw null check on the forwarded ref).

For consistency with the codebase, route these through isDefined() (e.g. isDefined(target.closest(...)), if (!isDefined(sidebarInteractionSafeAreaElement)), else if (isDefined(ref))).

✅ Quality: Test files call render() directly in it() without shared helper

📄 src/components/layout/global-navigation/tests/GlobalNavigationPrimary-test.tsx:37-51 📄 src/components/layout/global-navigation/tests/GlobalNavigationPrimary-test.tsx:73-87 📄 src/components/layout/sidebar-navigation/tests/SidebarNavigation-test.tsx:25-39 📄 src/components/layout/sidebar-navigation/tests/SidebarNavigation-test.tsx:53-67
Per the repo testing convention, test files should define a local setup*/render* helper that wraps render() and returns { user, ...rest }, rather than calling render() directly inside it() blocks. Layout-test.tsx follows this (new setupLayoutWithSidebar), but the modified tests in GlobalNavigationPrimary-test.tsx and SidebarNavigation-test.tsx still inline render(<LayoutContext.Provider value={{...}}>...) in each case, duplicating the large context-value object. Consider extracting a shared helper (e.g. setupGlobalNavigationPrimary({ contextOverrides })) that provides the default LayoutContextShape and merges overrides, reducing duplication and easing future context-shape changes.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/components/layout/sidebar-navigation/SidebarNavigation.tsx:62

• setHasSidebar(true) is triggered from a React effect, so the first paint renders with hasSidebar=false (hiding the sidebar + trigger button even when the sidebar should start docked). Using useLayoutEffect here would avoid that visible flicker/blank sidebar column on mount.

  useEffect(() => {
    setHasSidebar(true);

    return () => {
      setHasSidebar(false);
    };
  }, [setHasSidebar]);

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/components/layout/sidebar-navigation/SidebarNavigation.tsx:68

• Setting hasSidebar via useEffect can cause a brief mount-time state mismatch where the layout reserves space for a docked sidebar but the sidebar stays hidden until the effect runs (because isSidebarOpen is gated by hasSidebar). Use useLayoutEffect here so the sidebar presence is registered before the browser paints.

  useEffect(() => {
    setHasSidebar(true);

    return () => {
      setHasSidebar(false);
    };
  }, [setHasSidebar]);

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.

[gitar-bot]

Code Review ✅ Approved 10 resolved / 10 findings

Replaces the collapsed sidebar with an undocked interaction model and consolidates state logic via useLayoutSidebarState. All previous findings regarding prop forwarding, helper usage, and testing patterns have been resolved.

✅ 10 resolved

✅ Quality: Raw truthiness/null guards instead of isDefined() helper

📄 src/components/layout/LayoutSidebarInteraction.ts:23-34 📄 src/components/layout/sidebar-navigation/SidebarNavigation.tsx:72-83
The repo convention requires using the isDefined() helper from ~common/helpers/types for all != null / !== undefined / conditional-render guards (the helper is already used across sibling files like SidebarNavigationBaseItem.tsx, LayoutSlots.tsx, HeaderBase.tsx). Several new code paths use raw checks instead:

• LayoutSidebarInteraction.ts:26 — target.closest(...) !== null (raw null check).
• LayoutSidebarInteraction.ts:34 — if (!sidebarInteractionSafeAreaElement) (raw truthiness guard on an HTMLElement | null).
• SidebarNavigation.tsx:78 — else if (ref !== null) (raw null check on the forwarded ref).

For consistency with the codebase, route these through isDefined() (e.g. isDefined(target.closest(...)), if (!isDefined(sidebarInteractionSafeAreaElement)), else if (isDefined(ref))).

✅ Quality: Test files call render() directly in it() without shared helper

📄 src/components/layout/global-navigation/tests/GlobalNavigationPrimary-test.tsx:37-51 📄 src/components/layout/global-navigation/tests/GlobalNavigationPrimary-test.tsx:73-87 📄 src/components/layout/sidebar-navigation/tests/SidebarNavigation-test.tsx:25-39 📄 src/components/layout/sidebar-navigation/tests/SidebarNavigation-test.tsx:53-67
Per the repo testing convention, test files should define a local setup*/render* helper that wraps render() and returns { user, ...rest }, rather than calling render() directly inside it() blocks. Layout-test.tsx follows this (new setupLayoutWithSidebar), but the modified tests in GlobalNavigationPrimary-test.tsx and SidebarNavigation-test.tsx still inline render(<LayoutContext.Provider value={{...}}>...) in each case, duplicating the large context-value object. Consider extracting a shared helper (e.g. setupGlobalNavigationPrimary({ contextOverrides })) that provides the default LayoutContextShape and merges overrides, reducing duplication and easing future context-shape changes.

✅ Quality: SidebarNavigation does not forward rest props to root

📄 src/components/layout/sidebar-navigation/SidebarNavigation.tsx:42-43 📄 src/components/layout/sidebar-navigation/SidebarNavigation.tsx:84-98
SidebarNavigation destructures only { ariaLabel, children, className, ref } and never collects or spreads the remaining props onto a DOM node. Sibling components refactored in this same commit forward their rest props (GlobalNavigationPrimary and Layout spread ...htmlProps, SidebarNavigationHeader spreads ...radixProps), but SidebarNavigation silently swallows any additional data-*, aria-*, or other valid HTML attributes a consumer might pass. Per the repo convention, collect the remainder as ...restProps and spread it onto the root element (e.g. SidebarNavigationContainer or SidebarNavigationWrapper).

✅ Quality: Use isDefined() instead of raw !== undefined guards

📄 src/components/layout/useLayoutSidebarState.ts:74 📄 src/components/layout/useLayoutSidebarState.ts:123
The repo convention (and a previously-raised finding on LayoutSidebarInteraction.ts) requires using the isDefined() helper from ~common/helpers/types for all truthiness / null-ish guards instead of raw === undefined / !== undefined / != null checks. Two new guards on blurTimeoutRef.current use raw !== undefined:

• line 74: if (blurTimeoutRef.current !== undefined)
• line 123: if (blurTimeoutRef.current !== undefined)

Switch these to isDefined(blurTimeoutRef.current) for consistency with the rest of the codebase.

✅ Edge Case: Overflow handoff surface has zero width and can't be hovered

📄 src/components/layout/LayoutSidebarInteraction.ts:26 📄 src/components/layout/global-navigation/GlobalNavigationPrimary.tsx:290-295 📄 src/components/layout/sidebar-navigation/SidebarNavigation.tsx:142-156
SIDEBAR_OVERFLOW_HANDOFF_WIDTH_TOKEN is set to 'dimension-width-0', which resolves to 0px. This token drives the width of GlobalNavigationSidebarOverflowHandoff (width: 0 → no hit area even with pointer-events: auto) and contributes + 0 to both SIDEBAR_TRIGGER_BRIDGE_WIDTH and the SidebarNavigationHandoffGuide clip-path (calc(100% - 0) = a plain rectangle). As a result the overflow handoff strip that is meant to bridge the pointer gap between the leading nav content and the sidebar has no effective surface, so the intended hover hand-off through that region cannot occur. If this is a deliberate placeholder token, consider adding a comment; otherwise point it at a non-zero dimension token so the handoff surface actually has width.

...and 5 more resolved from earlier reviews

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
97.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube