Skip to content

[MISC] Enhance sidebar UX to expand on hover#1768

Merged
chandrasekharan-zipstack merged 25 commits intomainfrom
feat/expandable-sidebar
Feb 27, 2026
Merged

[MISC] Enhance sidebar UX to expand on hover#1768
chandrasekharan-zipstack merged 25 commits intomainfrom
feat/expandable-sidebar

Conversation

@chandrasekharan-zipstack
Copy link
Copy Markdown
Contributor

@chandrasekharan-zipstack chandrasekharan-zipstack commented Jan 29, 2026

What

  • Enhanced the sidebar to expand on hover with a pin/unpin toggle
  • Replaced pin icon with conventional >> chevron (matches Notion, Jira, Linear patterns)
  • Hidden scrollbar for cleaner appearance while preserving scroll
  • Added defensive guards for optional plugin imports (prevents crashes without cloud plugins)
  • Updated sample.env for Vite migration

Why

  • Previously had a button that had to be clicked, not intuitive enough and involves friction
  • Pin icon implied auto-hide behavior; chevrons are the standard convention for sidebar expand/collapse
  • Scrollbar was visually noisy on the narrow sidebar
  • Plugin imports were crashing when cloud plugins were unavailable

How

  • Added flex layout styles for .ant-layout-sider-children wrapper
  • Wrapped content in .sidebar-content-wrapper with min-height: 0 for proper flex scrolling
  • Implemented pin toggle with localStorage persistence
  • Added mouse enter/leave handlers for auto-collapse when unpinned
  • Replaced PushpinFilled/PushpinOutlined with single DoubleRightOutlined icon
  • Chevron turns blue when pinned, stays gray when unpinned (no hover color change)
  • Toggle only visible when sidebar is expanded; no tooltip for cleaner UX
  • Hidden scrollbar via scrollbar-width: none + ::-webkit-scrollbar { display: none }
  • Added optional chaining and nullish coalescing to plugin store/export access
  • Renamed REACT_APP_* env vars to VITE_* in sample.env

Can this PR break any existing features. If yes, please list possible items. If no, please explain why.

No. Changes are isolated to sidebar styling and behavior. Collapse functionality is preserved and improved. Plugin guards are additive (nullish coalescing with empty object fallback).

Notes on Testing

  • Test sidebar scroll when expanded (scrollbar should be invisible)
  • Test pin toggle and localStorage persistence
  • Test auto-collapse on mouse leave when unpinned
  • Test chevron visibility: hidden when collapsed, visible when expanded
  • Test chevron color: gray when unpinned, blue when pinned
  • Test all sidebar navigation items still work correctly
  • Verify app loads without cloud plugins (no crashes)

Screenshots

image

Checklist

I have read and understood the Contribution Guidelines.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 29, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds hover-to-expand sidebar with a persisted pin toggle, 300ms collapse debounce, safe localStorage helpers, CSS scroll/flex wrappers with a fixed bottom pin area, and moves collapse control into SideNavBar (now requires a setCollapsed prop).

Changes

Cohort / File(s) Summary
SideNavBar component
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx
Adds isPinned persisted via localStorage helpers, a 300ms collapse timeout (useRef), handleMouseEnter/handleMouseLeave, togglePin, optional chaining guards, pin button UI (tooltip + Pushpin icons), ARIA/test ids, and requires setCollapsed prop in PropTypes.
SideNavBar styles
frontend/src/components/navigations/side-nav-bar/SideNavBar.css
Introduces flex wrapper for sider children, scrollable content container, fixed bottom pin area with hover/pinned states, overflow/height and collapse-aware rules, pin icon styling and pinned state styles.
LocalStorage helpers
frontend/src/helpers/localStorage.js
New safe localStorage utilities: isLocalStorageAvailable(), getLocalStorageValue(key, defaultValue), and setLocalStorageValue(key, value) with graceful degradation for SSR/tests and error handling.
PageLayout component
frontend/src/layouts/page-layout/PageLayout.jsx
Removes inline collapse toggle UI and icon/button imports, initializes/persists collapsed via getLocalStorageValue/setLocalStorageValue, and passes setCollapsed into SideNavBar (collapse toggling delegated to SideNavBar).
PageLayout styles
frontend/src/layouts/page-layout/PageLayout.css
Removes the .collapse_btn CSS block related to the removed collapse toggle button.

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant Sider as SideNavBar
    participant Layout as PageLayout
    participant Storage as LocalStorage

    User->>Sider: mouse enter (hover)
    Sider->>Layout: call setCollapsed(false)
    Note right of Sider: expanded UI shown
    alt user clicks pin
        User->>Sider: click pin toggle
        Sider->>Storage: setLocalStorageValue("sidebarPinned", true/false)
        Sider->>Layout: ensure setCollapsed(false)
    else hover leave while unpinned
        User->>Sider: mouse leave
        Sider->>Sider: start 300ms collapse timer
        Sider->>Layout: call setCollapsed(true) after timeout
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: enhancing sidebar UX with hover expansion, which is the primary objective reflected throughout the changeset.
Description check ✅ Passed The pull request description is comprehensive and covers all required sections: What, Why, How, breaking changes analysis, testing notes, screenshots, and a completed checklist.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/expandable-sidebar

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.

@chandrasekharan-zipstack chandrasekharan-zipstack changed the title [MISC] Fix sidebar scroll and add pin tooltip [MISC] Enhance sidebar UX to expand on hover Jan 29, 2026
Copy link
Copy Markdown
Contributor

@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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx (1)

196-211: Conditional hook calls violate React's rules of hooks.

The hooks inside the try/catch and if blocks at lines 199-201 and 208-210 are called conditionally, which violates React's hooks rules. Hooks must be called in the same order on every render—conditional calls lead to unpredictable behavior and bugs.

Since this appears to be pre-existing code (only the trailing comma formatting changed), consider addressing it in a follow-up. The typical fix is to call the hook unconditionally and handle the absence of the store gracefully:

// Call unconditionally, default to a no-op selector if store unavailable
const unstractSubscriptionPlan = unstractSubscriptionPlanStore?.useUnstractSubscriptionPlanStore?.(
  (state) => state?.unstractSubscriptionPlan
) ?? null;
🤖 Fix all issues with AI agents
In `@frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx`:
- Around line 180-186: The collapse timeout started in handleMouseLeave (stored
on collapseTimeoutRef.current) isn't cleared on unmount; add a useEffect cleanup
that clears any pending timeout with clearTimeout(collapseTimeoutRef.current)
and optionally nulls collapseTimeoutRef.current to avoid calling setCollapsed
after the component has unmounted; ensure this effect runs once (empty deps) and
also consider clearing the timeout whenever isPinned changes or when collapsing
manually so setCollapsed won't be invoked on an unmounted component.

Copy link
Copy Markdown
Contributor

@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

🤖 Fix all issues with AI agents
In `@frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx`:
- Around line 162-164: The JSON.parse(localStorage.getItem("sidebarPinned"))
call can throw on corrupted data; wrap the read/parse in a try-catch inside the
SideNavBar component (where isPinned and setIsPinned are declared) so that if
JSON.parse throws you fall back to false (or a safe default) and optionally
clear the bad value from localStorage; update the initialization logic for
isPinned to use the safe parsed value from the try-catch and keep setIsPinned
usage the same.

Prevents calling setCollapsed after component unmounts

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add try-catch for localStorage parsing to handle corrupted data
- Split timeout cleanup into separate useEffects for clearer unmount handling

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Copy link
Copy Markdown
Contributor

@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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx (1)

224-239: ⚠️ Potential issue | 🟠 Major

Fix conditional hook calls to comply with React Rules of Hooks.

Both useUnstractSubscriptionPlanStore and useSelectedProductStore are called conditionally (lines 225 and 235). This violates React's Rules of Hooks: hooks must be called unconditionally at the top level of a component. If store availability changes between renders, the hook call order becomes inconsistent, causing bugs. Use a safe wrapper to ensure the hooks are always called.

🔧 Proposed fix (safe wrapper + unconditional call)
-  try {
-    if (unstractSubscriptionPlanStore?.useUnstractSubscriptionPlanStore) {
-      unstractSubscriptionPlan =
-        unstractSubscriptionPlanStore?.useUnstractSubscriptionPlanStore(
-          (state) => state?.unstractSubscriptionPlan,
-        );
-    }
-  } catch (error) {
-    // Do nothing
-  }
-
-  if (selectedProductStore?.useSelectedProductStore) {
-    selectedProduct = selectedProductStore.useSelectedProductStore(
-      (state) => state?.selectedProduct,
-    );
-  }
+  const useSafeUnstractSubscriptionPlanStore =
+    unstractSubscriptionPlanStore?.useUnstractSubscriptionPlanStore ??
+    ((selector) => selector?.({}));
+  const useSafeSelectedProductStore =
+    selectedProductStore?.useSelectedProductStore ?? ((selector) => selector?.({}));
+
+  try {
+    unstractSubscriptionPlan = useSafeUnstractSubscriptionPlanStore(
+      (state) => state?.unstractSubscriptionPlan,
+    );
+  } catch {
+    // Do nothing
+  }
+
+  selectedProduct = useSafeSelectedProductStore(
+    (state) => state?.selectedProduct,
+  );

Copy link
Copy Markdown
Contributor

@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

🤖 Fix all issues with AI agents
In `@frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx`:
- Around line 157-179: The getSafeLocalStorageValue and SideNavBar useEffect
assume localStorage is always available—calling localStorage.removeItem in the
catch and localStorage.setItem in the effect can throw in SSR/tests or when
storage is blocked; add guards and safe wrappers: update
getSafeLocalStorageValue to first check typeof localStorage !== "undefined" and
only call getItem/removeItem when available (avoid calling removeItem
unconditionally in the catch), create a safeSetLocalStorage helper used in the
useEffect that wraps setItem in try/catch and no-ops when storage is unavailable
or errors, and replace direct localStorage calls in SideNavBar with these guards
(referencing getSafeLocalStorageValue, safeSetLocalStorage, and the useEffect
that persists sidebarPinned).
🧹 Nitpick comments (1)
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx (1)

553-571: Prefer a semantic button for the pin control (with aria-pressed).

♻️ Suggested refactor
-      <Tooltip title={isPinned ? "Unpin sidebar" : "Keep expanded"}>
-        <div
-          className="sidebar-pin-container"
-          onClick={togglePin}
-          onKeyDown={(e) => {
-            if (e.key === "Enter" || e.key === " ") {
-              e.preventDefault();
-              togglePin();
-            }
-          }}
-          role="button"
-          tabIndex={0}
-        >
+      <Tooltip title={isPinned ? "Unpin sidebar" : "Keep expanded"}>
+        <button
+          type="button"
+          className="sidebar-pin-container"
+          onClick={togglePin}
+          aria-pressed={isPinned}
+          aria-label={isPinned ? "Unpin sidebar" : "Keep sidebar expanded"}
+        >
           {isPinned ? (
             <PushpinFilled className="sidebar-pin-icon pinned" />
           ) : (
             <PushpinOutlined className="sidebar-pin-icon" />
           )}
-        </div>
+        </button>
       </Tooltip>

- Create clearCollapseTimeout helper to eliminate duplication
- Combine 3 useEffects into 1 for localStorage, pin change, and unmount
- Simplify handleMouseEnter and handleMouseLeave

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Copy link
Copy Markdown
Contributor

@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

🤖 Fix all issues with AI agents
In `@frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx`:
- Around line 538-556: Replace the non-semantic div used for the pin control
with a native button element: in the block rendering the pin control (where
Tooltip wraps the element, and components/symbols involved include togglePin,
isPinned, PushpinFilled, PushpinOutlined, and the "sidebar-pin-container" /
"sidebar-pin-icon" classes), change the div to a <button type="button">, remove
role="button", tabIndex and the onKeyDown keyboard handling (the native button
handles Enter/Space), keep the onClick calling togglePin, preserve Tooltip,
className and conditional icon rendering, and add an explicit accessible name
via aria-label that reflects state (e.g., aria-label={isPinned ? "Unpin sidebar"
: "Keep expanded"}).

Menu items now have data-testid attributes (e.g., sidebar-workflows,
sidebar-api-deployments) that persist regardless of collapsed state,
allowing UI tests to find elements without relying on visible text.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Guard localStorage for SSR/tests with isLocalStorageAvailable check
- Wrap all localStorage operations in try-catch
- Replace div with native <button> for pin toggle (accessibility)
- Add aria-pressed and aria-label for screen reader support
- Update CSS to reset button default styles

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Copy link
Copy Markdown
Contributor

@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

🤖 Fix all issues with AI agents
In `@frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx`:
- Around line 194-229: The sidebar can remain collapsed when isPinned is loaded
true because useEffect only clears timeouts; update the isPinned change handling
to force expansion: inside the useEffect watching isPinned (the effect that
calls setSafeLocalStorageValue), when isPinned is true call setCollapsed(false)
(and clearCollapseTimeout via clearCollapseTimeout()) so any persisted pinned
state immediately expands; also ensure togglePin (which flips isPinned) keeps
the existing behavior of clearing timeouts and calling setCollapsed(false) when
newPinned is true. Reference functions/vars: useEffect (dependency [isPinned]),
setSafeLocalStorageValue, clearCollapseTimeout, setCollapsed, togglePin,
setIsPinned, collapseTimeoutRef.

Replace native button with Ant Design Button component
for consistency with rest of codebase.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Call setCollapsed(false) in useEffect when isPinned is true to ensure
  sidebar expands on initial load with persisted pinned state
- Add optional chaining to data iteration and item property access

Co-Authored-By: Claude Opus 4.5 <[email protected]>
…oltip

- Hide chevron on collapsed rail (hover-to-expand is sufficient)
- Always use >> instead of swapping between << and >>
- Remove tooltip for cleaner UX, matching Notion/Linear conventions

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Rename REACT_APP_* vars to VITE_*, remove CRA-specific HMR config,
and set WDS_SOCKET_PORT=80 for Traefik proxy compatibility.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Prevents crashes when cloud plugins are unavailable by adding
optional chaining and nullish coalescing to plugin store/export access.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Icon color only changes on click (gray ↔ blue), not on hover,
for a cleaner interaction.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Resolve merge conflict in SideNavBar.jsx and SideNavBar.css
- Use replaceAll over replace for global regex patterns (SideNavBar)
- Remove unused catch parameter in TopNavBar exception handler
- Refactor dropdown menu items to declarative array instead of multiple push calls

Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Fix SideNavBar.css formatting error (CI blocker)
- Use block statements in localStorage helpers
- Remove unused catch parameters in PersistentLogin and useSessionValid

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@github-actions
Copy link
Copy Markdown
Contributor

Frontend Lint Report (Biome)

All checks passed! No linting or formatting issues found.

@sonarqubecloud
Copy link
Copy Markdown

@chandrasekharan-zipstack chandrasekharan-zipstack merged commit 40dd724 into main Feb 27, 2026
7 checks passed
@chandrasekharan-zipstack chandrasekharan-zipstack deleted the feat/expandable-sidebar branch February 27, 2026 07:44
vishnuszipstack added a commit that referenced this pull request Feb 27, 2026
The sidebar hover UX refactor in #1768 inadvertently removed all HITL
navigation code that was added in #1804 (UN-3240). This restores the
REVIEW section with role-based popover menu items.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
vishnuszipstack added a commit that referenced this pull request Feb 27, 2026
Add back .sidebar-antd-icon CSS that was also removed by #1768. This
styles the antd FileProtectOutlined icon to match the sidebar theme
(grey by default, white on hover/active).

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Deepak-Kesavan pushed a commit that referenced this pull request Feb 27, 2026
)

* [FIX] Restore HITL sidebar navigation removed by sidebar refactor

The sidebar hover UX refactor in #1768 inadvertently removed all HITL
navigation code that was added in #1804 (UN-3240). This restores the
REVIEW section with role-based popover menu items.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* [FIX] Restore HITL sidebar icon styling removed by sidebar refactor

Add back .sidebar-antd-icon CSS that was also removed by #1768. This
styles the antd FileProtectOutlined icon to match the sidebar theme
(grey by default, white on hover/active).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
athul-rs added a commit that referenced this pull request Mar 2, 2026
- Fix HITL import path: manual_review_v2 → pluggable_apps.manual_review_v2
- Fix HITL model: ManualReviewEntity → HITLQueue with _base_manager
- Fix pages_processed: resolve org string identifier from UUID for PageUsage
- Add HITL metrics to live_series endpoint and summary
- Restore metrics dashboard sidebar nav removed by sidebar refactor (#1768)

Co-Authored-By: Claude Opus 4.6 <[email protected]>
athul-rs added a commit that referenced this pull request Mar 2, 2026
- Fix HITL import path: manual_review_v2 → pluggable_apps.manual_review_v2
- Fix HITL model: ManualReviewEntity → HITLQueue with _base_manager
- Fix pages_processed: resolve org string identifier from UUID for PageUsage
- Add HITL metrics to live_series endpoint and summary
- Restore metrics dashboard sidebar nav removed by sidebar refactor (#1768)

Co-Authored-By: Claude Opus 4.6 <[email protected]>
athul-rs added a commit that referenced this pull request Mar 2, 2026
- Fix HITL import path: manual_review_v2 → pluggable_apps.manual_review_v2
- Fix HITL model: ManualReviewEntity → HITLQueue with _base_manager
- Fix pages_processed: resolve org string identifier from UUID for PageUsage
- Add HITL metrics to live_series endpoint and summary
- Restore metrics dashboard sidebar nav removed by sidebar refactor (#1768)

Co-Authored-By: Claude Opus 4.6 <[email protected]>
athul-rs added a commit that referenced this pull request Mar 2, 2026
- Fix HITL import path: manual_review_v2 → pluggable_apps.manual_review_v2
- Fix HITL model: ManualReviewEntity → HITLQueue with _base_manager
- Fix pages_processed: resolve org string identifier from UUID for PageUsage
- Add HITL metrics to live_series endpoint and summary
- Restore metrics dashboard sidebar nav removed by sidebar refactor (#1768)

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Deepak-Kesavan added a commit that referenced this pull request Mar 2, 2026
* UN-1798: Fix dashboard metrics - HITL, pages processed, sidebar nav

- Fix HITL import path: manual_review_v2 → pluggable_apps.manual_review_v2
- Fix HITL model: ManualReviewEntity → HITLQueue with _base_manager
- Fix pages_processed: resolve org string identifier from UUID for PageUsage
- Add HITL metrics to live_series endpoint and summary
- Restore metrics dashboard sidebar nav removed by sidebar refactor (#1768)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Deepak K <[email protected]>
Deepak-Kesavan added a commit that referenced this pull request Mar 2, 2026
Tag was removed in PR #1768 (sidebar UX refactor). Re-adds it using
the existing el.tag pattern already present in the sidebar renderer.
athul-rs added a commit that referenced this pull request Mar 2, 2026
- Fix HITL import path: manual_review_v2 → pluggable_apps.manual_review_v2
- Fix HITL model: ManualReviewEntity → HITLQueue with _base_manager
- Fix pages_processed: resolve org string identifier from UUID for PageUsage
- Add HITL metrics to live_series endpoint and summary
- Restore metrics dashboard sidebar nav removed by sidebar refactor (#1768)

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Deepak-Kesavan added a commit that referenced this pull request Mar 2, 2026
…1817)

Tag was removed in PR #1768 (sidebar UX refactor). Re-adds it using
the existing el.tag pattern already present in the sidebar renderer.
athul-rs added a commit that referenced this pull request Mar 3, 2026
* UN-1798: Fix dashboard metrics - HITL, pages processed, sidebar nav

- Fix HITL import path: manual_review_v2 → pluggable_apps.manual_review_v2
- Fix HITL model: ManualReviewEntity → HITLQueue with _base_manager
- Fix pages_processed: resolve org string identifier from UUID for PageUsage
- Add HITL metrics to live_series endpoint and summary
- Restore metrics dashboard sidebar nav removed by sidebar refactor (#1768)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Deepak K <[email protected]>
athul-rs pushed a commit that referenced this pull request Mar 3, 2026
…1817)

Tag was removed in PR #1768 (sidebar UX refactor). Re-adds it using
the existing el.tag pattern already present in the sidebar renderer.
athul-rs added a commit that referenced this pull request Mar 4, 2026
- Fix HITL import path: manual_review_v2 → pluggable_apps.manual_review_v2
- Fix HITL model: ManualReviewEntity → HITLQueue with _base_manager
- Fix pages_processed: resolve org string identifier from UUID for PageUsage
- Add HITL metrics to live_series endpoint and summary
- Restore metrics dashboard sidebar nav removed by sidebar refactor (#1768)

Co-Authored-By: Claude Opus 4.6 <[email protected]>
athul-rs added a commit that referenced this pull request Mar 4, 2026
- Fix HITL import path: manual_review_v2 → pluggable_apps.manual_review_v2
- Fix HITL model: ManualReviewEntity → HITLQueue with _base_manager
- Fix pages_processed: resolve org string identifier from UUID for PageUsage
- Add HITL metrics to live_series endpoint and summary
- Restore metrics dashboard sidebar nav removed by sidebar refactor (#1768)

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Deepak-Kesavan pushed a commit that referenced this pull request Mar 4, 2026
* UN-1798: Fix dashboard metrics - HITL, pages processed, sidebar nav

- Fix HITL import path: manual_review_v2 → pluggable_apps.manual_review_v2
- Fix HITL model: ManualReviewEntity → HITLQueue with _base_manager
- Fix pages_processed: resolve org string identifier from UUID for PageUsage
- Add HITL metrics to live_series endpoint and summary
- Restore metrics dashboard sidebar nav removed by sidebar refactor (#1768)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* UN-3154 [FEAT] Card based layout to list Pipelines and API Deployments (#1769)

* UN-3154 [FEAT] Add execution status tracking and list enhancements

- Add WorkflowExecution.get_last_run_statuses() method to fetch last N
  executions with PARTIAL_SUCCESS computed dynamically
- Add last_5_run_statuses, run_count, last_run_time to API deployment serializer
- Add last_5_run_statuses, next_run_time to pipeline serializer
- Add source/destination instance names to pipeline representation
- Enable pagination (CustomPagination) for both pipelines and deployments
- Add search filter by name for both endpoints

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FEAT] Add reusable CardGridView component

- Create CardGridView widget for displaying items in card/list layouts
- Add CardItem component with expand/collapse, status badges, actions
- Add LoadingSkeleton for loading states
- Support pagination, search, scroll restoration, and card configuration
- Add dateFormatter helper for consistent date formatting

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FEAT] Refactor API Deployments to use CardGridView

- Add ApiDeploymentCardConfig for card layout configuration
- Refactor ApiDeployment to use CardGridView with pagination
- Update api-deployments-service with pagination and search support
- Refactor Body component to use CardGridView instead of Table
- Add search functionality to Header component
- Update Layout to pass card configuration and pagination props

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FEAT] Refactor Pipelines to use CardGridView

- Add PipelineCardConfig with StatusPills component for execution status
- Display source/destination connector icons and instance names
- Show last 5 run statuses with clickable navigation to execution logs
- Add schedule display, next run time, and run count
- Support pagination and search functionality

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FEAT] Improve navigation with scroll restoration

- Update MenuLayout to forward scrollToCardId state on back navigation
- Pass scrollToCardId from workflow links in card configs
- Add back route with state to ExecutionLogs for scroll restoration
- Remove redundant back button from DetailedLogs header
- Add previousRoute and previousRouteState props to ToolNavBar

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] Address CodeRabbit review comments

- Remove unused variables: type, filteredData, copyUrl, handleStatusRefresh,
  getPipelineData, sessionDetails
- Wrap cronstrue.toString() in try-catch for invalid cron expressions
- Use current pagination when refreshing after pipeline enable/disable
- Use String() coercion for type-safe ID comparison in CardGridView
- Update PropTypes to accept string or number for IDs
- Improve shortenApiEndpoint to strip query/hash and handle edge cases
- Merge state properly in MenuLayout back navigation

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] Address SonarQube issues and reduce code duplication

- Fix HIGH severity: Remove orphaned setFilteredData references in Pipelines.jsx
- Fix LOW severity: Add keyboard listeners to clickable spans for accessibility
  - PipelineCardConfig.jsx: Add role, tabIndex, onKeyDown to status badge
  - CardItem.jsx: Add keyboard handlers to action spans and expand chevron
- Reduce code duplication between ApiDeploymentCardConfig and PipelineCardConfig
  - Extract shared components to CardFieldComponents.jsx:
    CardActionBox, OwnerFieldRow, LastRunFieldRow, Last5RunsFieldRow,
    WorkflowFieldRow, ApiEndpointSection, CardHeaderRow
  - Net reduction: -289 lines in existing files

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] Address SonarCloud ReDoS security hotspot

Replace regex /\/+$/ with iterative string manipulation to strip
trailing slashes, avoiding potential ReDoS concerns flagged by
SonarCloud security scanner.

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] Fix function argument mismatch in clearFileMarkers

Update clearFileMarkers to accept optional workflowId parameter
to match the call site that passes pipeline.workflow_id.

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] Fix SonarQube issues and improve code quality

- Remove unused useSessionStore import in DetailedLogs.jsx
- Add search term persistence across pagination in Pipelines.jsx
- Fix stale closure in pipelineCardConfig useMemo dependencies
- Add keyboard accessibility (role, tabIndex, onKeyDown) to action icons
- Guard against undefined orgName in WorkflowFieldRow link rendering

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] Address SonarCloud code quality issues

- Fix array index keys in PipelineCardConfig.jsx and LoadingSkeleton.jsx
- Replace role="button" with native <button> elements for accessibility
- Fix text contrast ratios to meet WCAG AA (4.5:1 minimum)
- Fix unexpected negated conditions in pagination handlers
- Use globalThis instead of window for better portability
- Add proper button reset styles in CSS

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] Revert status pill colors and fix pipeline sorting

- Revert status badge colors to vibrant originals (pre-SonarCloud fix)
- Fix pipeline list sorting with nulls_last for last_run_time
- Add index fallback to React key in StatusPills to prevent duplicates
- Simplify enableDisablePipeline by removing unnecessary .then()
- Remove unused pagination deps from useMemo

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] Restore full list context on back navigation from logs

- Pass listContext (page, pageSize, searchTerm) through StatusPills navigation
- Restore pagination and search state in useEffect before scrolling
- Add listContext prop to Last5RunsFieldRow and StatusPills components
- Update useMemo dependencies to include pagination state

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] Replace native elements with AntD components per PR review

- Replace native buttons with AntD Button type="text" in CardActionBox
- Replace native button with AntD Button for expand chevron in CardItem
- Use AntD Space for status badges container in StatusPills
- Use AntD Tag instead of native button/span for status badges
- Fix require() inside component body → static import in Pipelines.jsx
- Update CSS to support both native and AntD component selectors
- Add missing listContext prop type to StatusPills

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] Address CodeRabbit PR review comments from #1769

- ApiDeployment: Use explicit null check instead of falsy check for data.count
- CardFieldComponents: Add URL scheme validation to prevent javascript: URLs
- PipelineCardConfig: Add keyboard accessibility to status badges
- CardGridView.css: Increase max-height from 200px to 500px for expanded content
- CardItem: Add setTimeout cleanup with clearTimeout to prevent memory leaks
- CardItem: Use isExpanded prop instead of expanded for grid mode support

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] ESLint eqeqeq error and add loading state to logs modal

- Fixed ESLint eqeqeq error: Changed != null to !== null && !== undefined check
- Added loading state to LogsModal to display spinner while fetching logs
- Updated fetchExecutionLogs to support optional setIsLoading callback
- Wired up isFetchingLogs state in both Pipelines and ApiDeployment components

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] Resolve card grid rendering issues and UI glitches

Fix card grid toggle flashing by memoizing CardItem and using optimistic updates. Prevent edit list from disappearing by using inline updates instead of refetch. Restore scroll position on back navigation using location.key. Remove unnecessary success alert on pipeline update. Keep share button always visible for consistency with workflows.

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] Remove ineffective CSS hover overrides for action icons

Remove background overrides that don't work due to AntD CSS-in-JS
specificity. Action icons will now show AntD's default hover state.

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [FIX] Backend sorting and share modal improvements

- Add backend ordering by last_run_time for API deployments using
  Subquery annotation with nulls_last for "Never" entries
- Fix share modal by passing deployment directly to avoid stale state
- Add robust response handling for different API response structures
- Remove Tooltip wrappers from action buttons to fix shadow issues

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* UN-3154 [REFACTOR] Extract shared hooks to reduce duplication in Pipelines and ApiDeployment

Extract duplicated logic into 4 reusable hooks (useExecutionLogs,
usePaginatedList, useScrollRestoration, useShareModal) and wire them
into both components. Also fixes a bug where ApiDeployment lost
searchTerm when changing pages or deleting items.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* MISC [FIX] Suppress eslint import/no-unresolved for Webpack url import

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* UN-3154 [FIX] Replace native elements with AntD components per PR review

Address Vishnu's review comments on PR #1769:
- Field rows: div/span → Flex + Typography.Text + Space
- img → AntD Avatar for WorkflowIcon
- a/span → Typography.Link / Typography.Text in ApiEndpointSection
- Header wrappers → Flex in CardHeaderRow and CardItem
- Actions container → Space in CardItem
- CSS: display block on subtitle, endpoint link targets .ant-typography-link

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* [FIX] Make Vite optional plugin emit throw instead of empty export

The empty module (`export default undefined`) made plugin imports
succeed silently, bypassing catch blocks and causing a login loop
in OSS mode.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* [MISC] Fix biome v2 lint and format issues in card grid view files

Sort imports, wrap single-line if/return in block statements,
add trailing commas, and fix CSS formatting.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* UN-3154 [REFACTOR] Replace divs/spans with AntD components, remove duplicate CSS

- Replaced raw HTML elements with Ant Design components (Flex, Space, Typography.Text, Avatar, Tag)
- Removed ~80 lines of custom CSS that duplicated AntD native functionality
- Fixed API endpoint Card padding/border specificity issues
- Fixed copy button padding in pipeline cards
- Addresses PR #1769 review feedback

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* UN-3154 [FIX] Fix tooltip icon colors, move lazy imports to top-level

Preserve green/red icon colors in status pill tooltips where
`color: inherit` was making them white on dark background.
Move unnecessary lazy imports to top-level across dashboard_metrics,
pipeline_v2 and workflow_manager modules.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* UN-3154 [FIX] Fix SonarCloud code smells in card grid view and related files

- Extract helpers to reduce function nesting depth (S2004)
- Replace nested ternary with nullish coalescing (S3358)
- Remove unused imports in DetailedLogs (S1128)
- Move PostHog try-catch into usePostHogEvents hook (S2486)
- Use globalThis over window (S7764)
- Fix non-native interactive element in CardItem (S6848)
- Use stable keys instead of array index in LoadingSkeleton (S6479)
- Improve contrast ratio for success status badge (S7924)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.5 <[email protected]>

* UN-1798: Merge subscription dashboard into metrics dashboard

- Move metrics dashboard from /metrics to /dashboard, replacing the old
  subscription-only dashboard page
- Add Subscription tab (cloud-only) showing plan info and daily page usage
- Add useSubscriptionUsage hook with overview data fallback
- Hide date range picker on subscription tab (not applicable)
- Add PlanBanner cloud-only guard (only show when subscription data exists)
- Make recent activity items clickable, navigating to execution logs
- Add back button in DetailedLogs to return to dashboard
- Remove old /metrics route and UnstractUsagePage import
- Consolidate sidebar to single "Dashboard" entry
- Improve cache key stability and add expired cache eviction

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* UN-1798: Address review feedback - accessibility, labels, cache fix

- Remove duplicate _get_hitl_queue_model function from services.py
- Make recent activity rows keyboard accessible (tabIndex, role, onKeyDown)
- Fix HITL chart legend labels to use proper casing
- Fix formatMetricName to uppercase known acronyms (API, ETL, LLM, HITL)
- Show Subscription tab unconditionally when component is available
- Treat malformed cache entries as expired during eviction

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* UN-1798: Replace LLM Usage by Workflow with Usage by Deployment

The old "LLM Usage" tab queried the usage table grouped by workflow_id,
but most usage rows have NULL workflow_id making the tab empty. This
replaces it with per-deployment-type sub-tabs (API, ETL, Task, Workflow)
that join through workflow_execution to resolve deployment names.

Backend:
- Add get_deployment_usage() with 4 deployment type queries
- Add deployment_type query param to workflow-token-usage endpoint
- Enforce 30-day max date range for deployment usage queries
- Include execution stats, pages processed, and date range per deployment

Frontend:
- Replace useWorkflowTokenUsage with useDeploymentUsage hook
- Rewrite LLMUsageTable as DeploymentUsageTable with sub-tabs
- Add per-deployment-type localStorage caching
- Show 30-day limit note, hide 90-day preset on deployment tab

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* UN-1798: Show error state for failed deployment usage fetches

Surface backend/network errors in DeploymentUsageTable instead of
silently showing "No data" empty state.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* UN-1798: Fix Biome formatting in LLMUsageTable

Collapse multi-line import to single line per Biome formatter.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* UN-1798: Show empty state when dashboard has no data

Display a centered empty state with action buttons (Create Workflow,
Open Prompt Studio) when all data sources return empty, instead of
showing individual empty cards for each chart.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* UN-1798: Align dashboard header and padding with other pages

- Remove icon from Dashboard header, use plain Typography (16px/600)
  matching ETL Pipelines, API Deployments, Prompt Studio pages
- Add horizontal padding to recent activity list items and usage table
  so content aligns with card headers

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* UN-1798: Fix SonarQube code smells across dashboard

- Backend: extract 4 helper methods from get_deployment_usage to reduce
  cognitive complexity from 52 to ~10 per method
- MetricsChart: replace() → replaceAll() for ES2021 compliance
- MetricsDashboard: flip negated ternary condition
- metricsCache: remove unused catch parameter, add explanatory comment
- useMetricsData: specify react-hooks/exhaustive-deps in eslint-disable

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Use ExecutionEntity enum instead of hardcoded deployment type strings

Replaces hardcoded "API", "ETL", "TASK", "WF" string literals with
ExecutionEntity enum values from workflow_manager.execution.enum,
as suggested in code review.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Use default model managers instead of _base_manager for view-context queries

Replace _base_manager.filter(organization_id=...) with .objects which
auto-filters by the current user's organization via
DefaultOrganizationManagerMixin. This applies to _get_deployment_names
and get_deployment_usage which are only called from HTTP request
context where UserContext is available.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Address PR review feedback for deployment usage endpoint

- Use ExecutionStatus.value instead of str() for enum comparisons
- Rename first_date/last_date to first_execution_at/last_execution_at
- Remove organization_id param from get_deployment_usage (org scoping
  handled by DefaultOrganizationManagerMixin via model managers)
- Move deployment_type validation and 30-day range cap into new
  DeploymentUsageQuerySerializer, using DRF ChoiceField for validation
- Remove manual error response construction in favor of DRF's
  raise_exception=True
- Update frontend table column to match renamed API field

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Address PR review feedback: query optimization, serializer validation, UI polish

- Replace Python-dict materialization with DB-level Subquery+OuterRef for pages aggregation
- Add range_truncated flag to API response; frontend conditionally shows 30-day notice
- Move inline styles to CSS classes in LLMUsageTable
- Add range_truncated to DeploymentUsageQuerySerializer

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix Sonar issues: reduce cognitive complexity, use replaceAll

- Refactor _build_exec_stats to reduce cognitive complexity from 18 to ~11
  using setdefault, dict-based status lookup, and min/max for timestamps
- Use String.replaceAll() instead of String.replace() in MetricsChart.jsx

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Fix deployment usage: restore _get_usage_queryset() for Usage queries

Usage.objects uses DefaultOrganizationManagerMixin which auto-filters by
UserContext.get_organization(). This can return empty results when the
org context doesn't match. Restore _get_usage_queryset() which uses
_base_manager to bypass this filter, matching the pattern used by all
other Usage queries in the service.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix UUID/varchar type mismatch in pages query

The Subquery approach in _get_pages_by_deployment used
OuterRef("run_id") (varchar) to match WorkflowFileExecution.id (UUID),
causing a PostgreSQL type mismatch error. Revert to the working
two-query approach that explicitly converts IDs to strings.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Chandrasekharan M <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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.

3 participants