Review: Simplify layout handling

code/core/src/manager-api/modules/shortcuts.ts

@@ -9,6 +9,7 @@ import { global } from '@storybook/global';
 
 import copy from 'copy-to-clipboard';
 
+import { isReviewManagerRoute } from '../../shared/review/routes.ts';
 import type { KeyboardEventLike } from '../lib/shortcut.ts';
 import { eventToShortcut, shortcutMatchesShortcut } from '../lib/shortcut.ts';
 import type { ModuleFn } from '../lib/types.tsx';
@@ -257,16 +258,23 @@ export const init: ModuleFn = ({ store, fullAPI, provider }) => {
 
     // warning: event might not have a full prototype chain because it may originate from the channel
     handleShortcutFeature(feature, event) {
+      const state = store.getState();
       const {
         ui: { enableShortcuts },
         storyId,
         refId,
         viewMode,
-      } = store.getState();
+      } = state;
       if (!enableShortcuts) {
         return;
       }
 
+      const isSidebarShortcutBlocked = isReviewManagerRoute(state.path, state.customQueryParams);
+      const isSidebarShortcutFeature = ['focusNav', 'search', 'toggleNav'].includes(feature);
+      if (isSidebarShortcutBlocked && isSidebarShortcutFeature) {
+        return;
+      }
+
       // Event.prototype.preventDefault is missing when received from the MessageChannel.
       if (event?.preventDefault) {
         event.preventDefault();

code/core/src/manager/components/layout/Layout.stories.tsx

@@ -514,8 +514,29 @@ export const MobileReview: Story = {
   args: {
     managerLayoutState: { ...defaultState, viewMode: 'review' },
   },
+  decorators: [
+    (Story) => (
+      <ManagerContext.Provider
+        value={{
+          ...mockManagerStore,
+          state: {
+            ...mockManagerStore.state,
+            path: '/review/',
+            viewMode: 'review',
+            customQueryParams: {},
+          },
+          api: {
+            ...mockManagerStore.api,
+            getIsNavShown: fn(() => true),
+          },
+        }}
+      >
+        <Story />
+      </ManagerContext.Provider>
+    ),
+  ],
   play: async ({ canvas }) => {
-    expect(canvas.getByLabelText('Open navigation menu')).toBeInTheDocument();
+    expect(canvas.queryByLabelText('Open navigation menu')).not.toBeInTheDocument();
     expect(canvas.getByTestId('preview')).toBeInTheDocument();
   },
 };

code/core/src/manager/components/layout/Layout.tsx

@@ -3,9 +3,11 @@ import React, { useEffect, useLayoutEffect, useState } from 'react';
 
 import type { API_Layout, API_ViewMode } from 'storybook/internal/types';
 
-import { useStorybookApi, type API } from 'storybook/manager-api';
+import { useStorybookApi, useStorybookState, type API } from 'storybook/manager-api';
 import { styled } from 'storybook/theming';
 
+import { isReviewManagerRoute } from '../../../shared/review/routes.ts';
+
 import { MEDIA_DESKTOP_BREAKPOINT, MINIMUM_CONTENT_WIDTH_PX } from '../../constants.ts';
 import { Notifications } from '../../container/Notifications.tsx';
 import { MobileNavigation } from '../mobile/navigation/MobileNavigation.tsx';
@@ -149,6 +151,9 @@ const OrderedMobileNavigation = styled(MobileNavigation)({
 export const Layout = ({ managerLayoutState, setManagerLayoutState, hasTab, ...slots }: Props) => {
   const { isDesktop, isMobile } = useLayout();
   const api = useStorybookApi();
+  const { path, customQueryParams } = useStorybookState();
+  const showSidebar =
+    (api.getIsNavShown?.() ?? true) && !isReviewManagerRoute(path, customQueryParams);
 
   const {
     navSize,
@@ -170,6 +175,7 @@ export const Layout = ({ managerLayoutState, setManagerLayoutState, hasTab, ...s
     <LayoutContainer
       panelPosition={managerLayoutState.panelPosition}
       showPanel={showPanel}
+      showSidebar={showSidebar}
       style={
         {
           '--nav-width': `${navSize}px`,
@@ -179,7 +185,7 @@ export const Layout = ({ managerLayoutState, setManagerLayoutState, hasTab, ...s
       }
     >
       <>
-        {isDesktop && (
+        {isDesktop && showSidebar && (
           <SidebarContainer
             navSize={navSize}
             sidebarMaxWidth={sidebarMaxWidth}
@@ -192,6 +198,7 @@ export const Layout = ({ managerLayoutState, setManagerLayoutState, hasTab, ...s
           <OrderedMobileNavigation
             menu={slots.slotSidebar}
             panel={slots.slotPanel}
+            showMenu={showSidebar}
             showPanel={showPanel}
           />
         )}
@@ -222,7 +229,8 @@ export const Layout = ({ managerLayoutState, setManagerLayoutState, hasTab, ...s
 const LayoutContainer = styled.div<{
   panelPosition: LayoutState['panelPosition'];
   showPanel: boolean;
-}>(({ panelPosition, showPanel }) => ({
+  showSidebar: boolean;
+}>(({ panelPosition, showPanel, showSidebar }) => ({
   width: '100%',
   height: ['100vh', '100dvh'],
   overflow: 'hidden',
@@ -233,21 +241,33 @@ const LayoutContainer = styled.div<{
   [MEDIA_DESKTOP_BREAKPOINT]: {
     display: 'grid',
     gap: 0,
-    // This uses CSS variables to prevent Emotion from generating a new CSS className for every possible value
-    gridTemplateColumns: `minmax(0, var(--nav-width)) minmax(${MINIMUM_CONTENT_WIDTH_PX}px, 1fr) minmax(0, var(--right-panel-width))`,
+    gridTemplateColumns: showSidebar
+      ? `minmax(0, var(--nav-width)) minmax(${MINIMUM_CONTENT_WIDTH_PX}px, 1fr) minmax(0, var(--right-panel-width))`
+      : `minmax(${MINIMUM_CONTENT_WIDTH_PX}px, 1fr) minmax(0, var(--right-panel-width))`,
     gridTemplateRows: `1fr minmax(0, var(--bottom-panel-height))`,
     gridTemplateAreas: (() => {
+      if (!showSidebar && !showPanel) {
+        return `"content content"
+                "content content"`;
+      }
+      if (!showSidebar && showPanel) {
+        if (panelPosition === 'right') {
+          return `"content panel"
+                  "content panel"`;
+        }
+        return `"content content"
+                "panel   panel"`;
+      }
       if (!showPanel) {
-        // showPanel is false by default when viewMode is not 'story', but can be overridden by the user
         return `"sidebar content content"
-                  "sidebar content content"`;
+                "sidebar content content"`;
       }
       if (panelPosition === 'right') {
         return `"sidebar content panel"
-                  "sidebar content panel"`;
+                "sidebar content panel"`;
       }
       return `"sidebar content content"
-                "sidebar panel   panel"`;
+              "sidebar panel   panel"`;
     })(),
   },
 }));

code/core/src/manager/components/mobile/navigation/MobileNavigation.tsx

@@ -1,5 +1,5 @@
-import React, { useRef } from 'react';
 import type { ComponentProps, FC } from 'react';
+import React, { useLayoutEffect, useRef } from 'react';
 
 import { Button } from 'storybook/internal/components';
 import type { API_IndexHash, API_Refs } from 'storybook/internal/types';
@@ -18,6 +18,7 @@ import { MobileMenuDrawer } from './MobileMenuDrawer.tsx';
 interface MobileNavigationProps {
   menu?: React.ReactNode;
   panel?: React.ReactNode;
+  showMenu?: boolean;
   showPanel: boolean;
 }
 
@@ -75,6 +76,7 @@ interface MobileBottomBarContentProps {
   setMobileMenuOpen: (isOpen: boolean) => void;
   isMobilePanelOpen: boolean;
   setMobilePanelOpen: (isOpen: boolean) => void;
+  showMenu: boolean;
   showPanel: boolean;
 }
 
@@ -91,6 +93,7 @@ const MobileBottomBarContent: FC<MobileBottomBarContentProps> = ({
   setMobileMenuOpen,
   isMobilePanelOpen,
   setMobilePanelOpen,
+  showMenu,
   showPanel,
 }) => {
   const headingId = useId();
@@ -105,17 +108,19 @@ const MobileBottomBarContent: FC<MobileBottomBarContentProps> = ({
       <h2 id={headingId} className="sb-sr-only">
         Navigation controls
       </h2>
-      <BottomBarButton
-        padding="small"
-        variant="ghost"
-        onClick={() => setMobileMenuOpen(!isMobileMenuOpen)}
-        ariaLabel="Open navigation menu"
-        aria-expanded={isMobileMenuOpen}
-        aria-controls="storybook-mobile-menu"
-      >
-        <MenuIcon />
-        <Text>{fullStoryName}</Text>
-      </BottomBarButton>
+      {showMenu && (
+        <BottomBarButton
+          padding="small"
+          variant="ghost"
+          onClick={() => setMobileMenuOpen(!isMobileMenuOpen)}
+          ariaLabel="Open navigation menu"
+          aria-expanded={isMobileMenuOpen}
+          aria-controls="storybook-mobile-menu"
+        >
+          <MenuIcon />
+          <Text>{fullStoryName}</Text>
+        </BottomBarButton>
+      )}
       <span className="sb-sr-only" aria-current="page">
         {fullStoryName}
       </span>
@@ -138,22 +143,31 @@ const MobileBottomBarContent: FC<MobileBottomBarContentProps> = ({
 export const MobileNavigation: FC<MobileNavigationProps & ComponentProps<typeof Container>> = ({
   menu,
   panel,
+  showMenu = true,
   showPanel,
   ...props
 }) => {
   const { isMobileMenuOpen, isMobilePanelOpen, setMobileMenuOpen, setMobilePanelOpen } =
     useLayout();
   const fullStoryName = useFullStoryName();
 
+  useLayoutEffect(() => {
+    if (!showMenu) {
+      setMobileMenuOpen(false);
+    }
+  }, [showMenu, setMobileMenuOpen]);
+
   return (
     <Container {...props}>
-      <MobileMenuDrawer
-        id="storybook-mobile-menu"
-        isOpen={isMobileMenuOpen}
-        onOpenChange={setMobileMenuOpen}
-      >
-        {menu}
-      </MobileMenuDrawer>
+      {showMenu && (
+        <MobileMenuDrawer
+          id="storybook-mobile-menu"
+          isOpen={isMobileMenuOpen}
+          onOpenChange={setMobileMenuOpen}
+        >
+          {menu}
+        </MobileMenuDrawer>
+      )}
 
       <MobileAddonsDrawer
         id="storybook-mobile-addon-panel"
@@ -163,13 +177,14 @@ export const MobileNavigation: FC<MobileNavigationProps & ComponentProps<typeof
         {panel}
       </MobileAddonsDrawer>
 
-      {!isMobilePanelOpen && (
+      {!isMobilePanelOpen && (showMenu || showPanel) && (
         <MobileBottomBarContent
           fullStoryName={fullStoryName}
           isMobileMenuOpen={isMobileMenuOpen}
           setMobileMenuOpen={setMobileMenuOpen}
           isMobilePanelOpen={isMobilePanelOpen}
           setMobilePanelOpen={setMobilePanelOpen}
+          showMenu={showMenu}
           showPanel={showPanel}
         />
       )}

code/core/src/manager/components/preview/Preview.tsx

@@ -13,6 +13,7 @@ import { Helmet } from 'react-helmet-async';
 import { Consumer, addons, merge, types, type Combo } from 'storybook/manager-api';
 
 import { useLandmark } from '../../hooks/useLandmark.ts';
+import { REVIEW_COLLECTION_QUERY_PARAM } from '../../../shared/review/routes.ts';
 import { ReviewToolbarHeader } from '../review/components/ReviewToolbarHeader.tsx';
 import { FramesRenderer } from './FramesRenderer.tsx';
 import { ToolbarComp } from './Toolbar.tsx';
@@ -59,6 +60,7 @@ const Preview = React.memo<PreviewProps>(function Preview(props) {
     tabs,
     wrappers,
     tabId,
+    queryParams,
   } = props;
 
   // SB11: remove code
@@ -90,6 +92,8 @@ const Preview = React.memo<PreviewProps>(function Preview(props) {
   const shouldScale = viewMode === 'story';
   const { showToolbar } = options;
   const customisedShowToolbar = api.getShowToolbarWithCustomisations(showToolbar);
+  const isReviewCollectionStory =
+    viewMode === 'story' && queryParams?.[REVIEW_COLLECTION_QUERY_PARAM] !== undefined;
 
   const previousStoryId = useRef(storyId);
 
@@ -128,7 +132,7 @@ const Preview = React.memo<PreviewProps>(function Preview(props) {
       )}
       <ZoomProvider shouldScale={shouldScale}>
         <S.PreviewContainer>
-          {customisedShowToolbar && viewMode === 'story' ? <ReviewToolbarHeader /> : null}
+          {customisedShowToolbar && isReviewCollectionStory ? <ReviewToolbarHeader /> : null}
           <ToolbarComp
             key="tools"
             isShown={customisedShowToolbar}

code/core/src/manager/components/preview/Toolbar.tsx

@@ -1,23 +1,24 @@
 import React, { useRef } from 'react';
 
 import { AbstractToolbar, Button, Separator, TabList } from 'storybook/internal/components';
-import { type Addon_BaseType, Addon_TypesEnum } from 'storybook/internal/types';
+import { Addon_TypesEnum, type Addon_BaseType } from 'storybook/internal/types';
 
 import { CloseIcon, ExpandIcon } from '@storybook/icons';
 
 import type { TabListState } from '@react-stately/tabs';
 import {
-  type API,
-  type Combo,
   Consumer,
-  type LeafEntry,
-  type State,
   addons,
   merge,
   types,
+  type API,
+  type Combo,
+  type LeafEntry,
+  type State,
 } from 'storybook/manager-api';
 import { styled } from 'storybook/theming';
 
+import { isReviewManagerRoute } from '../../../shared/review/routes.ts';
 import { useLandmark } from '../../hooks/useLandmark.ts';
 import { useLayout } from '../layout/LayoutProvider.tsx';
 import type { PreviewProps } from './utils/types.tsx';
@@ -32,6 +33,7 @@ const fullScreenMapper = ({ api, state }: Combo) => {
     shortcut: api.getShortcutKeys().fullScreen,
     hasPanel: Object.keys(api.getElements(Addon_TypesEnum.PANEL)).length > 0,
     singleStory: state.singleStory,
+    isReviewRoute: isReviewManagerRoute(state.path, state.customQueryParams),
   };
 };
 
@@ -50,7 +52,8 @@ export const fullScreenTool: Addon_BaseType = {
 
     return (
       <Consumer filter={fullScreenMapper}>
-        {({ toggle, isFullscreen, shortcut, hasPanel, singleStory }) =>
+        {({ toggle, isFullscreen, shortcut, hasPanel, singleStory, isReviewRoute }) =>
+          !isReviewRoute &&
           (!singleStory || (singleStory && hasPanel)) && (
             <Button
               key="full"

code/core/src/manager/components/preview/tools/menu.tsx

@@ -9,11 +9,12 @@ import { Consumer, types } from 'storybook/manager-api';
 import type { Combo } from 'storybook/manager-api';
 
 import { focusableUIElements } from '../../../../manager-api/modules/layout.ts';
+import { isReviewManagerRoute } from '../../../../shared/review/routes.ts';
 
 const menuMapper = ({ api, state }: Combo) => ({
   isVisible: api.getIsNavShown(),
   singleStory: state.singleStory,
-  viewMode: state.viewMode,
+  isReviewCollectionStory: isReviewManagerRoute(state.path, state.customQueryParams),
   showSidebar: async (forceFocus: boolean) => {
     api.toggleNav(true);
     api.focusOnUIElement(focusableUIElements.sidebarRegion, {
@@ -32,9 +33,10 @@ export const menuTool: Addon_BaseType = {
   render: () => {
     return (
       <Consumer filter={menuMapper}>
-        {({ isVisible, showSidebar, singleStory }) =>
+        {({ isVisible, showSidebar, singleStory, isReviewCollectionStory }) =>
           !singleStory &&
-          !isVisible && (
+          !isVisible &&
+          !isReviewCollectionStory && (
             <>
               <Button
                 padding="small"