Merge pull request #26360 from storybookjs/shilman/26334-fix-viewport…

…-edit

Viewport: Fix editing when default viewport is set

code/addons/viewport/src/Tool.tsx

@@ -136,7 +136,7 @@ export const ViewportTool: FC = memo(
 
     useEffect(() => {
       registerShortcuts(api, globals, updateGlobals, Object.keys(viewports));
-    }, [viewports, globals.viewport]);
+    }, [viewports, globals, globals.viewport, updateGlobals, api]);
 
     useEffect(() => {
       const defaultRotated = defaultOrientation === 'landscape';
@@ -150,7 +150,18 @@ export const ViewportTool: FC = memo(
           viewportRotated: defaultRotated,
         });
       }
-    }, [defaultOrientation, defaultViewport, globals, updateGlobals]);
+      // NOTE: we don't want to re-run this effect when `globals` changes
+      // due to https://github.com/storybookjs/storybook/issues/26334
+      //
+      // Also, this *will* rerun every time you change story as the parameter is briefly `undefined`.
+      // This behaviour is intentional, if a bit of a happy accident in implementation.
+      //
+      // Ultimately this process of "locking in" a parameter value should be
+      // replaced by https://github.com/storybookjs/storybook/discussions/23347
+      // or something similar.
+      //
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [defaultOrientation, defaultViewport, updateGlobals]);
 
     const item =
       list.find((i) => i.id === globals.viewport) ||

code/e2e-tests/addon-viewport.spec.ts

@@ -40,4 +40,26 @@ test.describe('addon-viewport', () => {
     // Compare the two widths
     await expect(adjustedDimensions?.width).not.toBe(originalDimensions?.width);
   });
+
+  test('viewport should be editable when a default viewport is set', async ({ page }) => {
+    const sbPage = new SbPage(page);
+
+    // Story parameters/selected is set to small mobile
+    await sbPage.navigateToStory('addons/viewport/parameters', 'selected');
+
+    // Measure the original dimensions of previewRoot
+    const originalDimensions = await sbPage.getCanvasBodyElement().boundingBox();
+    await expect(originalDimensions?.width).toBeDefined();
+
+    // Manually select "large mobile" and give it time to adjust
+    await sbPage.selectToolbar('[title="Change the size of the preview"]', '#list-item-mobile2');
+    await new Promise((r) => setTimeout(r, 200));
+
+    // Measure the adjusted dimensions of previewRoot after clicking the mobile item.
+    const adjustedDimensions = await sbPage.getCanvasBodyElement().boundingBox();
+    await expect(adjustedDimensions?.width).toBeDefined();
+
+    // Compare the two widths
+    await expect(adjustedDimensions?.width).not.toBe(originalDimensions?.width);
+  });
 });