From 3b72b84c0546ba5e67de9ea7528bdf0cf4cb1f6a Mon Sep 17 00:00:00 2001 From: NSTKrishna Date: Wed, 16 Sep 2026 16:31:43 +0530 Subject: [PATCH 1/4] fix(dashboard): decouple edit session from sidebar visibility Introduce DashboardLayoutContext to communicate sidebar visibility, mobile status, and open/close controls to child components (e.g. WidgetPicker). Add internal isSidebarVisible state to DashboardLayout so closing the picker minimizes the panel without ending the edit session (isSidebarOpen remains true). Provide showReopenFab and controlled sidebarVisible / onSidebarVisibilityChange props on DashboardLayout. Update WidgetPicker to auto-hide redundant close button inside mobile BottomSheet while keeping it on desktop, with showCloseButton override. Wire WidgetPicker close to context.closeSidebar if onClose is not passed. Fixes #1845 Signed-off-by: NSTKrishna --- src/__testing__/DashboardLayout.test.tsx | 315 ++++++++++++++++++ src/__testing__/WidgetPicker.test.tsx | 180 ++++++++++ .../DashboardLayout/DashboardLayout.tsx | 169 +++++++--- .../DashboardLayoutContext.tsx | 27 ++ src/custom/DashboardLayout/index.tsx | 5 + src/custom/WidgetPicker/WidgetPicker.tsx | 34 +- src/index.tsx | 5 +- 7 files changed, 691 insertions(+), 44 deletions(-) create mode 100644 src/__testing__/DashboardLayout.test.tsx create mode 100644 src/__testing__/WidgetPicker.test.tsx create mode 100644 src/custom/DashboardLayout/DashboardLayoutContext.tsx diff --git a/src/__testing__/DashboardLayout.test.tsx b/src/__testing__/DashboardLayout.test.tsx new file mode 100644 index 000000000..abed1d77c --- /dev/null +++ b/src/__testing__/DashboardLayout.test.tsx @@ -0,0 +1,315 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import React from 'react'; +import { DashboardLayout } from '../custom/DashboardLayout/DashboardLayout'; +import { DashboardLayoutContext, useDashboardLayoutContext } from '../custom/DashboardLayout/DashboardLayoutContext'; +import { SistentThemeProvider } from '../theme'; + +// --------------------------------------------------------------------------- +// Mock breakpoint / media query +// --------------------------------------------------------------------------- + +let mockIsMobile = false; + +jest.mock('@mui/material', () => ({ + ...jest.requireActual('@mui/material'), + useMediaQuery: () => mockIsMobile +})); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +const WIDGETS =
Widget Gallery
; + +function renderLayout( + props: Partial> & { + isSidebarOpen: boolean; + } +) { + return render( + + +
Dashboard Content
+ + + ); +} + +// --------------------------------------------------------------------------- +// Mobile tests +// --------------------------------------------------------------------------- + +describe('DashboardLayout – mobile', () => { + beforeEach(() => { + mockIsMobile = true; + }); + afterAll(() => { + mockIsMobile = false; + }); + + it('renders BottomSheet title and no reopen FAB when isSidebarOpen=true', () => { + renderLayout({ isSidebarOpen: true }); + // BottomSheet renders its title text + expect(screen.queryByText('Widget Picker')).not.toBeNull(); + // No reopen FAB while sheet is open + expect(screen.queryByLabelText('Open Widget Picker')).toBeNull(); + }); + + it('does not render BottomSheet or FAB when isSidebarOpen=false', () => { + renderLayout({ isSidebarOpen: false }); + // BottomSheet title is not rendered + expect(screen.queryByText('Widget Picker')).toBeNull(); + // No reopen FAB either + expect(screen.queryByLabelText('Open Widget Picker')).toBeNull(); + }); + + it('shows FAB when BottomSheet is closed and keeps edit mode active', () => { + renderLayout({ isSidebarOpen: true }); + // BottomSheet has its own close button labelled 'Close' + const closeBtn = screen.getByLabelText('Close'); + fireEvent.click(closeBtn); + // FAB appears so the user can reopen (sheet is minimized, but MUI Dialog + // keeps DOM content mounted — so we check the FAB, not the close button) + expect(screen.queryByLabelText('Open Widget Picker')).not.toBeNull(); + }); + + it('reopens BottomSheet when FAB is clicked', () => { + renderLayout({ isSidebarOpen: true }); + // Close the sheet first + fireEvent.click(screen.getByLabelText('Close')); + // FAB is now visible + expect(screen.queryByLabelText('Open Widget Picker')).not.toBeNull(); + // Click the reopen FAB + fireEvent.click(screen.getByLabelText('Open Widget Picker')); + // FAB disappears because sheet is open again + expect(screen.queryByLabelText('Open Widget Picker')).toBeNull(); + }); + + it('hides FAB when showReopenFab=false even when sheet is minimized', () => { + renderLayout({ isSidebarOpen: true, showReopenFab: false }); + fireEvent.click(screen.getByLabelText('Close')); + expect(screen.queryByLabelText('Open Widget Picker')).toBeNull(); + }); + + it('provides isSheet=true to sidebarContent via context', () => { + let capturedContext: ReturnType = null; + + function ContextCapture() { + capturedContext = useDashboardLayoutContext(); + return null; + } + + renderLayout({ isSidebarOpen: true, sidebarContent: }); + + expect(capturedContext).not.toBeNull(); + expect(capturedContext?.isSheet).toBe(true); + expect(capturedContext?.isMobile).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// Desktop tests +// --------------------------------------------------------------------------- + +describe('DashboardLayout – desktop', () => { + beforeEach(() => { + mockIsMobile = false; + }); + + it('renders sidebar content and no FAB when isSidebarOpen=true', () => { + renderLayout({ isSidebarOpen: true }); + expect(screen.queryByText('Widget Gallery')).not.toBeNull(); + expect(screen.queryByLabelText('Open Widget Picker')).toBeNull(); + }); + + it('does not render sidebar or FAB when isSidebarOpen=false', () => { + renderLayout({ isSidebarOpen: false }); + expect(screen.queryByText('Widget Gallery')).toBeNull(); + expect(screen.queryByLabelText('Open Widget Picker')).toBeNull(); + }); + + it('provides isSheet=false to sidebarContent via context', () => { + let capturedContext: ReturnType = null; + + function ContextCapture() { + capturedContext = useDashboardLayoutContext(); + return null; + } + + // Must use WIDGETS to have sidebarContent that renders AND the context capture + renderLayout({ + isSidebarOpen: true, + sidebarContent: ( + <> + + {WIDGETS} + + ) + }); + + expect(capturedContext).not.toBeNull(); + expect(capturedContext?.isSheet).toBe(false); + expect(capturedContext?.isMobile).toBe(false); + }); + + it('shows FAB and hides sidebar when closeSidebar is called from context', () => { + let capturedContext: ReturnType = null; + + function MinimizeButton() { + capturedContext = useDashboardLayoutContext(); + return ( + + ); + } + + renderLayout({ isSidebarOpen: true, sidebarContent: }); + + // sidebar is open: close button visible, no FAB + expect(screen.queryByLabelText('minimize')).not.toBeNull(); + expect(screen.queryByLabelText('Open Widget Picker')).toBeNull(); + + // Minimize the sidebar via context + fireEvent.click(screen.getByLabelText('minimize')); + + // sidebar is hidden: minimize button gone, reopen FAB visible + expect(screen.queryByLabelText('minimize')).toBeNull(); + expect(screen.queryByLabelText('Open Widget Picker')).not.toBeNull(); + }); + + it('reopens desktop sidebar when FAB is clicked after minimizing', () => { + let capturedContext: ReturnType = null; + + function MinimizeButton() { + capturedContext = useDashboardLayoutContext(); + return ( + + ); + } + + renderLayout({ isSidebarOpen: true, sidebarContent: }); + + // Minimize sidebar + fireEvent.click(screen.getByLabelText('minimize')); + expect(screen.queryByLabelText('Open Widget Picker')).not.toBeNull(); + + // Click reopen FAB + fireEvent.click(screen.getByLabelText('Open Widget Picker')); + // Sidebar is back: minimize button visible, no FAB + expect(screen.queryByLabelText('minimize')).not.toBeNull(); + expect(screen.queryByLabelText('Open Widget Picker')).toBeNull(); + }); + + it('respects controlled sidebarVisible=true prop', () => { + const onSidebarVisibilityChange = jest.fn(); + render( + + +
Dashboard
+ + + ); + + expect(screen.queryByText('Widget Gallery')).not.toBeNull(); + expect(screen.queryByLabelText('Open Widget Picker')).toBeNull(); + }); + + it('respects controlled sidebarVisible=false prop (shows FAB)', () => { + const onSidebarVisibilityChange = jest.fn(); + render( + + +
Dashboard
+ + + ); + + expect(screen.queryByText('Widget Gallery')).toBeNull(); + expect(screen.queryByLabelText('Open Widget Picker')).not.toBeNull(); + }); + + it('calls onSidebarVisibilityChange when FAB is clicked in controlled mode', () => { + const onSidebarVisibilityChange = jest.fn(); + render( + + +
Dashboard
+ + + ); + + fireEvent.click(screen.getByLabelText('Open Widget Picker')); + expect(onSidebarVisibilityChange).toHaveBeenCalledWith(true); + }); +}); + +// --------------------------------------------------------------------------- +// Context value tests +// --------------------------------------------------------------------------- + +describe('DashboardLayoutContext – standalone usage', () => { + it('useDashboardLayoutContext returns null when used outside DashboardLayout', () => { + let ctxValue: ReturnType = undefined as unknown as null; + + function Probe() { + ctxValue = useDashboardLayoutContext(); + return null; + } + + render(); + expect(ctxValue).toBeNull(); + }); + + it('DashboardLayoutContext.Provider propagates value correctly', () => { + let ctxValue: ReturnType = null; + const mockClose = jest.fn(); + const mockOpen = jest.fn(); + + function Probe() { + ctxValue = useDashboardLayoutContext(); + return null; + } + + render( + + + + ); + + expect(ctxValue?.isMobile).toBe(false); + expect(ctxValue?.isSheet).toBe(false); + expect(ctxValue?.isSidebarVisible).toBe(true); + + ctxValue?.closeSidebar(); + expect(mockClose).toHaveBeenCalledTimes(1); + + ctxValue?.openSidebar(); + expect(mockOpen).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/__testing__/WidgetPicker.test.tsx b/src/__testing__/WidgetPicker.test.tsx new file mode 100644 index 000000000..22bbaba19 --- /dev/null +++ b/src/__testing__/WidgetPicker.test.tsx @@ -0,0 +1,180 @@ +import { render, screen, fireEvent } from '@testing-library/react'; +import React from 'react'; +import { WidgetPicker, type WidgetPickerProps } from '../custom/WidgetPicker/WidgetPicker'; +import { DashboardLayoutContext, type DashboardLayoutContextValue } from '../custom/DashboardLayout/DashboardLayoutContext'; +import { SistentThemeProvider } from '../theme'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +const noop = () => {}; + +const WIDGETS: WidgetPickerProps['widgetsToAdd'] = [ + { key: 'chart', title: 'Chart Widget' }, + { key: 'table', title: 'Table Widget' } +]; + +function makeContext( + overrides: Partial = {} +): DashboardLayoutContextValue { + return { + isMobile: false, + isSheet: false, + isSidebarVisible: true, + closeSidebar: noop, + openSidebar: noop, + ...overrides + }; +} + +function renderPicker( + props: Partial = {}, + context?: DashboardLayoutContextValue | null +) { + const element = ( + + + + ); + + if (context === undefined) { + // No context wrapper — standalone usage + return render(element); + } + + return render( + + {element} + + ); +} + +// --------------------------------------------------------------------------- +// Close button auto-detection +// --------------------------------------------------------------------------- + +describe('WidgetPicker – close button auto-detection', () => { + it('renders close button when standalone with onClose provided', () => { + const handleClose = jest.fn(); + renderPicker({ onClose: handleClose }); + expect(screen.queryByLabelText('Close widget picker')).not.toBeNull(); + }); + + it('does not render close button when standalone with no onClose and no context', () => { + renderPicker(); + expect(screen.queryByLabelText('Close widget picker')).toBeNull(); + }); + + it('suppresses close button when embedded in BottomSheet (isSheet=true)', () => { + renderPicker( + { onClose: noop }, + makeContext({ isSheet: true }) + ); + expect(screen.queryByLabelText('Close widget picker')).toBeNull(); + }); + + it('renders close button when embedded in desktop sidebar (isSheet=false) with onClose', () => { + renderPicker( + { onClose: noop }, + makeContext({ isSheet: false }) + ); + expect(screen.queryByLabelText('Close widget picker')).not.toBeNull(); + }); + + it('renders close button when embedded in desktop sidebar with only closeSidebar in context', () => { + const closeSidebar = jest.fn(); + renderPicker( + {}, + makeContext({ isSheet: false, closeSidebar }) + ); + expect(screen.queryByLabelText('Close widget picker')).not.toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// showCloseButton explicit override +// --------------------------------------------------------------------------- + +describe('WidgetPicker – showCloseButton explicit override', () => { + it('showCloseButton=true forces close button even inside BottomSheet', () => { + renderPicker( + { showCloseButton: true }, + makeContext({ isSheet: true }) + ); + expect(screen.queryByLabelText('Close widget picker')).not.toBeNull(); + }); + + it('showCloseButton=false hides close button even when onClose is provided', () => { + renderPicker({ onClose: noop, showCloseButton: false }); + expect(screen.queryByLabelText('Close widget picker')).toBeNull(); + }); + + it('showCloseButton=false hides close button even in desktop context with closeSidebar', () => { + const closeSidebar = jest.fn(); + renderPicker( + { showCloseButton: false }, + makeContext({ isSheet: false, closeSidebar }) + ); + expect(screen.queryByLabelText('Close widget picker')).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// Close handler wiring +// --------------------------------------------------------------------------- + +describe('WidgetPicker – close handler wiring', () => { + it('calls onClose when close button is clicked (standalone)', () => { + const handleClose = jest.fn(); + renderPicker({ onClose: handleClose }); + fireEvent.click(screen.getByLabelText('Close widget picker')); + expect(handleClose).toHaveBeenCalledTimes(1); + }); + + it('calls onClose (not closeSidebar) when both are available', () => { + const handleClose = jest.fn(); + const closeSidebar = jest.fn(); + renderPicker( + { onClose: handleClose }, + makeContext({ isSheet: false, closeSidebar }) + ); + fireEvent.click(screen.getByLabelText('Close widget picker')); + expect(handleClose).toHaveBeenCalledTimes(1); + expect(closeSidebar).not.toHaveBeenCalled(); + }); + + it('calls closeSidebar when onClose is not provided but context has closeSidebar', () => { + const closeSidebar = jest.fn(); + renderPicker( + {}, + makeContext({ isSheet: false, closeSidebar }) + ); + fireEvent.click(screen.getByLabelText('Close widget picker')); + expect(closeSidebar).toHaveBeenCalledTimes(1); + }); +}); + +// --------------------------------------------------------------------------- +// Widget list rendering +// --------------------------------------------------------------------------- + +describe('WidgetPicker – widget list', () => { + it('renders all widget titles', () => { + renderPicker(); + expect(screen.queryByText('Chart Widget')).not.toBeNull(); + expect(screen.queryByText('Table Widget')).not.toBeNull(); + }); + + it('shows empty-state message when widgetsToAdd is empty', () => { + renderPicker({ widgetsToAdd: [] }); + expect(screen.queryByText('All widgets added to the layout.')).not.toBeNull(); + }); + + it('calls onAddWidget with correct args when add button is clicked', () => { + const handleAddWidget = jest.fn(); + renderPicker({ onAddWidget: handleAddWidget }); + fireEvent.click(screen.getByLabelText('Add Chart Widget widget')); + expect(handleAddWidget).toHaveBeenCalledWith({ title: 'Chart Widget' }, 'chart'); + }); +}); diff --git a/src/custom/DashboardLayout/DashboardLayout.tsx b/src/custom/DashboardLayout/DashboardLayout.tsx index ba19afb68..8dfb97d9c 100644 --- a/src/custom/DashboardLayout/DashboardLayout.tsx +++ b/src/custom/DashboardLayout/DashboardLayout.tsx @@ -1,8 +1,9 @@ -import React, { useState, useEffect, useRef } from 'react'; +import React, { useState, useEffect, useRef, useCallback } from 'react'; import { Box, Fab } from '../../base'; import { AddIcon } from '../../icons/Add'; import { useTheme, useMediaQuery } from '../../theme'; import { BottomSheet } from '../BottomSheet'; +import { DashboardLayoutContext } from './DashboardLayoutContext'; export interface DashboardLayoutProps { /** The main dashboard content (typically the React-Grid-Layout) */ @@ -25,7 +26,7 @@ export interface DashboardLayoutProps { /** Optional sticky top offset for the sidebar (useful if page has a top navbar) */ sidebarTopOffset?: string | number; - /** Optional fixed height for the sticky sidebar. Defaults to 100dvh */ + /** Optional fixed height for the sticky sidebar. Defaults to `calc(100dvh - )`. */ sidebarHeight?: string | number; /** Background color for the mobile bottom sheet header */ @@ -33,6 +34,36 @@ export interface DashboardLayoutProps { /** Text color for the mobile bottom sheet header */ headerTextColor?: string; + + /** + * Controlled sidebar-panel visibility. + * When provided, DashboardLayout becomes a controlled component for the + * panel's open/minimized state and will not manage `sidebarVisible` + * internally. Pair with `onSidebarVisibilityChange`. + */ + sidebarVisible?: boolean; + + /** + * Initial panel visibility when running uncontrolled. + * Ignored when `sidebarVisible` is provided. + * Defaults to `true` (panel starts open). + */ + defaultSidebarVisible?: boolean; + + /** + * Called when the panel's open/minimized state changes. + * Receives the next value (`true` = expanded, `false` = minimized). + */ + onSidebarVisibilityChange?: (visible: boolean) => void; + + /** + * Whether to render the floating reopen FAB when the sidebar panel is + * minimized while edit mode is still active. + * Defaults to `true`. + * Set to `false` when the host application provides its own toolbar button + * that can reopen the panel. + */ + showReopenFab?: boolean; } export const DashboardLayout: React.FC = ({ @@ -42,34 +73,62 @@ export const DashboardLayout: React.FC = ({ sidebarTitle = 'Widget Picker', sidebarWidth = { xs: '100%', md: '350px' }, sidebarTopOffset = '0', - sidebarHeight = '100dvh', + sidebarHeight, headerBackgroundColor, - headerTextColor + headerTextColor, + sidebarVisible: controlledVisible, + defaultSidebarVisible = true, + onSidebarVisibilityChange, + showReopenFab = true, }) => { const theme = useTheme(); // We use the 'md' breakpoint (900px default) to switch between mobile and desktop layout const isMobile = useMediaQuery(theme.breakpoints.down('md')); - // isSheetVisible is independently owned by DashboardLayout: - // - resets to true whenever Edit Mode (isSidebarOpen) transitions OFF → ON - // - can be set to false by the user dismissing the sheet (FAB appears instead) - // - set to false when Edit Mode turns OFF - // This two-dimension model prevents the sheet from re-opening on every - // isSidebarOpen change after the user has intentionally minimized it. - const [isSheetVisible, setIsSheetVisible] = useState(isSidebarOpen); + // isSidebarVisible represents "panel expanded" (true) vs "panel minimized" (false). + // This is independent of isSidebarOpen (the edit-session flag). + // In uncontrolled mode, we manage visibility internally. + // In controlled mode (sidebarVisible prop provided), the caller drives it. + const isControlled = controlledVisible !== undefined; + const [internalVisible, setInternalVisible] = useState(defaultSidebarVisible); + + const isSidebarVisible = isControlled ? (controlledVisible as boolean) : internalVisible; + + const setSidebarVisible = useCallback( + (next: boolean) => { + if (!isControlled) { + setInternalVisible(next); + } + onSidebarVisibilityChange?.(next); + }, + [isControlled, onSidebarVisibilityChange] + ); + const prevIsSidebarOpen = useRef(isSidebarOpen); useEffect(() => { if (isSidebarOpen && !prevIsSidebarOpen.current) { - // Edit Mode just turned ON → pop the sheet open - setIsSheetVisible(true); + // Edit Mode just turned ON → expand the panel + setSidebarVisible(true); } if (!isSidebarOpen) { - // Edit Mode turned OFF → close the sheet and hide the FAB - setIsSheetVisible(false); + // Edit Mode turned OFF → collapse the panel + setSidebarVisible(false); } prevIsSidebarOpen.current = isSidebarOpen; - }, [isSidebarOpen]); + }, [isSidebarOpen, setSidebarVisible]); + + const closeSidebar = useCallback(() => setSidebarVisible(false), [setSidebarVisible]); + const openSidebar = useCallback(() => setSidebarVisible(true), [setSidebarVisible]); + + // Derive sidebar height: if sidebarTopOffset is a non-zero string or number, + // use calc(100dvh - ) so the sidebar never pushes content off viewport. + const resolvedSidebarHeight = + sidebarHeight !== undefined + ? sidebarHeight + : sidebarTopOffset && sidebarTopOffset !== '0' && sidebarTopOffset !== 0 + ? `calc(100dvh - ${typeof sidebarTopOffset === 'number' ? `${sidebarTopOffset}px` : sidebarTopOffset})` + : '100dvh'; return ( @@ -79,24 +138,28 @@ export const DashboardLayout: React.FC = ({ {isSidebarOpen && isMobile && ( <> - setIsSheetVisible(false)} - title={sidebarTitle} - maxHeight="50vh" - headerBackgroundColor={headerBackgroundColor} - headerTextColor={headerTextColor} + - {sidebarContent} - + + {sidebarContent} + + - {/* FAB appears when Edit Mode is active but the sheet has been minimized, + {/* FAB appears when Edit Mode is active but the panel has been minimized, letting users rearrange the dashboard and pull the picker back up. */} - {!isSheetVisible && ( + {!isSidebarVisible && showReopenFab && ( setIsSheetVisible(true)} + onClick={openSidebar} sx={(fabTheme) => ({ position: 'fixed', bottom: 24, @@ -111,19 +174,43 @@ export const DashboardLayout: React.FC = ({ )} {isSidebarOpen && !isMobile && ( - - {sidebarContent} - + <> + {isSidebarVisible ? ( + + + {sidebarContent} + + + ) : ( + showReopenFab && ( + ({ + position: 'fixed', + bottom: 24, + right: 24, + zIndex: fabTheme.zIndex.drawer, + })} + > + + + ) + )} + )} ); diff --git a/src/custom/DashboardLayout/DashboardLayoutContext.tsx b/src/custom/DashboardLayout/DashboardLayoutContext.tsx new file mode 100644 index 000000000..e32cf589b --- /dev/null +++ b/src/custom/DashboardLayout/DashboardLayoutContext.tsx @@ -0,0 +1,27 @@ +import { createContext, useContext } from 'react'; + +export interface DashboardLayoutContextValue { + /** Whether the dashboard is currently at the mobile breakpoint (below 'md'). */ + isMobile: boolean; + /** Whether the sidebarContent is currently rendered inside the mobile BottomSheet. */ + isSheet: boolean; + /** Whether the sidebar panel is currently visible/expanded (not minimized). */ + isSidebarVisible: boolean; + /** Minimize the sidebar panel (shows the reopen FAB without ending edit mode). */ + closeSidebar: () => void; + /** Expand the sidebar panel. */ + openSidebar: () => void; +} + +export const DashboardLayoutContext = createContext(null); + +/** + * Consume the DashboardLayoutContext value. + * + * Returns `null` when called outside a DashboardLayout — consumers (e.g. + * WidgetPicker) should treat `null` as "no layout context available" and fall + * back to their standalone behaviour. + */ +export function useDashboardLayoutContext(): DashboardLayoutContextValue | null { + return useContext(DashboardLayoutContext); +} diff --git a/src/custom/DashboardLayout/index.tsx b/src/custom/DashboardLayout/index.tsx index 552a08c05..9ca808709 100644 --- a/src/custom/DashboardLayout/index.tsx +++ b/src/custom/DashboardLayout/index.tsx @@ -1,2 +1,7 @@ export { DashboardLayout } from './DashboardLayout'; export type { DashboardLayoutProps } from './DashboardLayout'; +export { + DashboardLayoutContext, + useDashboardLayoutContext, + type DashboardLayoutContextValue +} from './DashboardLayoutContext'; diff --git a/src/custom/WidgetPicker/WidgetPicker.tsx b/src/custom/WidgetPicker/WidgetPicker.tsx index 15aca9415..b6e331e76 100644 --- a/src/custom/WidgetPicker/WidgetPicker.tsx +++ b/src/custom/WidgetPicker/WidgetPicker.tsx @@ -3,6 +3,7 @@ import { Box, IconButton, Stack, Typography } from '../../base'; import { AddIcon, CloseIcon } from '../../icons'; import { useTheme } from '../../theme'; import type { SxProps, Theme } from '@mui/material'; +import { useDashboardLayoutContext } from '../DashboardLayout/DashboardLayoutContext'; export interface WidgetItem { key: string; @@ -21,6 +22,16 @@ export interface WidgetPickerProps { /** Optional callback to close the picker (renders a Close icon if provided) */ onClose?: () => void; + /** + * Explicit override for close-button visibility. + * - `true` → always render the close button. + * - `false` → never render it. + * - `undefined` (default) → auto-detect: hidden when embedded in a + * DashboardLayout BottomSheet (to avoid a duplicate 'X'), shown when + * embedded in the desktop sticky sidebar or used standalone. + */ + showCloseButton?: boolean; + /** Custom background color for the header. Defaults to theme.palette.background.default */ headerBackgroundColor?: string; @@ -35,11 +46,30 @@ export const WidgetPicker: React.FC = ({ widgetsToAdd, onAddWidget, onClose, + showCloseButton, headerBackgroundColor, headerTextColor, containerSx = {}, }) => { const theme = useTheme(); + const layoutContext = useDashboardLayoutContext(); + + // Resolve whether to show the close button. + // Explicit `showCloseButton` prop always wins. + // Otherwise: hide when embedded in the mobile BottomSheet (the sheet already + // has its own close/drag affordance), show when on desktop sidebar or standalone. + const shouldShowClose = + showCloseButton !== undefined + ? showCloseButton + : !layoutContext?.isSheet && Boolean(onClose ?? layoutContext?.closeSidebar); + + const handleClose = () => { + if (onClose) { + onClose(); + } else { + layoutContext?.closeSidebar?.(); + } + }; return ( = ({ Widgets - {onClose && ( - + {shouldShowClose && ( + )} diff --git a/src/index.tsx b/src/index.tsx index 24a87e1a1..500c9791e 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -35,7 +35,10 @@ export { export { DashboardLayout, - type DashboardLayoutProps + type DashboardLayoutProps, + DashboardLayoutContext, + useDashboardLayoutContext, + type DashboardLayoutContextValue } from './custom/DashboardLayout'; // Same nested-barrel dts-drop quirk as FeedbackButton above: UniversalFilter // (and its FilterColumn / UniversalFilterProps types) reaches the entry only From ceeb2240d093b52363715aa9e9e7856a5ca1cb7f Mon Sep 17 00:00:00 2001 From: NSTKrishna Date: Wed, 16 Sep 2026 16:37:26 +0530 Subject: [PATCH 2/4] test(dashboard): assert sidebar height derivation from sidebarTopOffset Adds tests verifying that DashboardLayout derives its desktop sidebar height as calc(100dvh - offset) when sidebarTopOffset is provided, respects explicit sidebarHeight overrides, and falls back to 100dvh. Covers #1843 Signed-off-by: NSTKrishna --- src/__testing__/DashboardLayout.test.tsx | 58 ++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/__testing__/DashboardLayout.test.tsx b/src/__testing__/DashboardLayout.test.tsx index abed1d77c..dd55d2ce3 100644 --- a/src/__testing__/DashboardLayout.test.tsx +++ b/src/__testing__/DashboardLayout.test.tsx @@ -312,4 +312,62 @@ describe('DashboardLayoutContext – standalone usage', () => { ctxValue?.openSidebar(); expect(mockOpen).toHaveBeenCalledTimes(1); }); + + describe('sidebar height derivation from sidebarTopOffset (#1843)', () => { + it('derives height as calc(100dvh - offset) when sidebarTopOffset is provided without sidebarHeight', () => { + renderLayout({ + isSidebarOpen: true, + sidebarTopOffset: '64px', + sidebarContent:
content
+ }); + + const container = screen.getByTestId('sidebar-child').parentElement; + const styles = window.getComputedStyle(container as Element); + expect(styles.height).toBe('calc(100dvh - 64px)'); + expect(styles.maxHeight).toBe('calc(100dvh - 64px)'); + expect(styles.top).toBe('64px'); + }); + + it('formats numeric sidebarTopOffset in pixels for height calc', () => { + renderLayout({ + isSidebarOpen: true, + sidebarTopOffset: 80, + sidebarContent:
content
+ }); + + const container = screen.getByTestId('sidebar-child').parentElement; + const styles = window.getComputedStyle(container as Element); + expect(styles.height).toBe('calc(100dvh - 80px)'); + expect(styles.maxHeight).toBe('calc(100dvh - 80px)'); + expect(styles.top).toBe('80px'); + }); + + it('respects explicit sidebarHeight when both are provided', () => { + renderLayout({ + isSidebarOpen: true, + sidebarTopOffset: '64px', + sidebarHeight: '500px', + sidebarContent:
content
+ }); + + const container = screen.getByTestId('sidebar-child').parentElement; + const styles = window.getComputedStyle(container as Element); + expect(styles.height).toBe('500px'); + expect(styles.maxHeight).toBe('500px'); + expect(styles.top).toBe('64px'); + }); + + it('defaults to 100dvh when sidebarTopOffset is 0 or not provided', () => { + renderLayout({ + isSidebarOpen: true, + sidebarTopOffset: '0', + sidebarContent:
content
+ }); + + const container = screen.getByTestId('sidebar-child').parentElement; + const styles = window.getComputedStyle(container as Element); + expect(styles.height).toBe('100dvh'); + expect(styles.maxHeight).toBe('100dvh'); + }); + }); }); From b88a9ef65ace32c40e88f7ccf3f063934a6ee74e Mon Sep 17 00:00:00 2001 From: NSTKrishna Date: Wed, 16 Sep 2026 20:46:11 +0530 Subject: [PATCH 3/4] fix(dashboard): only collapse sidebar on edit-mode transition Guard the sidebar collapse effect in DashboardLayout to check prevIsSidebarOpen.current, preventing premature collapse and duplicate notifications on initial mount or callback re-creation when edit mode is inactive. Update WidgetPicker onClose docstring to accurately reflect its role as the preferred close handler rather than the close button visibility gate. Signed-off-by: NSTKrishna --- src/__testing__/DashboardLayout.test.tsx | 17 +++++++++++++++++ src/custom/DashboardLayout/DashboardLayout.tsx | 2 +- src/custom/WidgetPicker/WidgetPicker.tsx | 5 ++++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/__testing__/DashboardLayout.test.tsx b/src/__testing__/DashboardLayout.test.tsx index dd55d2ce3..62e02bcee 100644 --- a/src/__testing__/DashboardLayout.test.tsx +++ b/src/__testing__/DashboardLayout.test.tsx @@ -259,6 +259,23 @@ describe('DashboardLayout – desktop', () => { fireEvent.click(screen.getByLabelText('Open Widget Picker')); expect(onSidebarVisibilityChange).toHaveBeenCalledWith(true); }); + + it('does not emit onSidebarVisibilityChange on initial mount when isSidebarOpen is false', () => { + const onSidebarVisibilityChange = jest.fn(); + render( + + +
Dashboard
+ + + ); + + expect(onSidebarVisibilityChange).not.toHaveBeenCalled(); + }); }); // --------------------------------------------------------------------------- diff --git a/src/custom/DashboardLayout/DashboardLayout.tsx b/src/custom/DashboardLayout/DashboardLayout.tsx index 8dfb97d9c..c14c55e22 100644 --- a/src/custom/DashboardLayout/DashboardLayout.tsx +++ b/src/custom/DashboardLayout/DashboardLayout.tsx @@ -111,7 +111,7 @@ export const DashboardLayout: React.FC = ({ // Edit Mode just turned ON → expand the panel setSidebarVisible(true); } - if (!isSidebarOpen) { + if (!isSidebarOpen && prevIsSidebarOpen.current) { // Edit Mode turned OFF → collapse the panel setSidebarVisible(false); } diff --git a/src/custom/WidgetPicker/WidgetPicker.tsx b/src/custom/WidgetPicker/WidgetPicker.tsx index b6e331e76..cf07466a0 100644 --- a/src/custom/WidgetPicker/WidgetPicker.tsx +++ b/src/custom/WidgetPicker/WidgetPicker.tsx @@ -19,7 +19,10 @@ export interface WidgetPickerProps { /** Callback when a widget is clicked to be added */ onAddWidget: (widget: Omit, key: string) => void; - /** Optional callback to close the picker (renders a Close icon if provided) */ + /** + * Optional callback to close the picker. When provided, takes precedence + * over the layout context's closeSidebar handler. + */ onClose?: () => void; /** From fd686429798113ca313c0bc8caf2b18483609c13 Mon Sep 17 00:00:00 2001 From: Krishna Gehlot Date: Thu, 17 Sep 2026 10:34:50 +0530 Subject: [PATCH 4/4] Clarify FAB behavior in BottomSheet test Update comment to clarify FAB behavior after closing BottomSheet. Signed-off-by: Krishna Gehlot --- src/__testing__/DashboardLayout.test.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/__testing__/DashboardLayout.test.tsx b/src/__testing__/DashboardLayout.test.tsx index 62e02bcee..b14090074 100644 --- a/src/__testing__/DashboardLayout.test.tsx +++ b/src/__testing__/DashboardLayout.test.tsx @@ -68,8 +68,7 @@ describe('DashboardLayout – mobile', () => { // BottomSheet has its own close button labelled 'Close' const closeBtn = screen.getByLabelText('Close'); fireEvent.click(closeBtn); - // FAB appears so the user can reopen (sheet is minimized, but MUI Dialog - // keeps DOM content mounted — so we check the FAB, not the close button) + // FAB appears immediately so the user can reopen, while the sheet transitions closed expect(screen.queryByLabelText('Open Widget Picker')).not.toBeNull(); });