Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 10.1.6

- Manager: Do not display non-existing shortcuts in the settings page - [#32711](https://github.com/storybookjs/storybook/pull/32711), thanks @DKER2!
- Preview: Enforce inert body if manager is focus-trapped - [#33186](https://github.com/storybookjs/storybook/pull/33186), thanks @Sidnioulz!
- Telemetry: Await pending operations in getLastEvents to prevent race conditions - [#33285](https://github.com/storybookjs/storybook/pull/33285), thanks @valentinpalkovic!
- UI: Fix keyboard navigation bug for "reset" option in `Select` - [#33268](https://github.com/storybookjs/storybook/pull/33268), thanks @Sidnioulz!

## 10.1.5

- Addon-Vitest: Isolate error reasons during postinstall - [#33295](https://github.com/storybookjs/storybook/pull/33295), thanks @valentinpalkovic!
Expand Down
13 changes: 12 additions & 1 deletion code/core/src/components/components/Modal/Modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import { deprecate } from 'storybook/internal/client-logger';
import type { DecoratorFunction } from 'storybook/internal/csf';

import { FocusScope } from '@react-aria/focus';
import { Overlay, UNSAFE_PortalProvider, useModalOverlay } from '@react-aria/overlays';
import {
Overlay,
UNSAFE_PortalProvider,
ariaHideOutside,
useModalOverlay,
} from '@react-aria/overlays';
import { mergeProps } from '@react-aria/utils';
import { useOverlayTriggerState } from '@react-stately/overlays';
import type { KeyboardEvent as RAKeyboardEvent } from '@react-types/shared';
Expand Down Expand Up @@ -169,6 +174,12 @@ function BaseModal({
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isMounted]);

useEffect(() => {
if (isMounted && (open || defaultOpen) && overlayRef.current) {
return ariaHideOutside([overlayRef.current], { shouldUseInert: true });
}
}, [isMounted, open, defaultOpen, overlayRef]);

if (!isMounted || status === 'exited' || status === 'unmounted') {
return null;
}
Expand Down
41 changes: 41 additions & 0 deletions code/core/src/components/components/Select/Select.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,47 @@ export const KeyboardSelectionMultiSS = meta.story({
play: kbMultiSelectionTest(' ', ' '),
});

export const FullArrowNavigation = meta.story({
play: async ({ canvas, step }) => {
const selectButton = await canvas.findByRole('button');
await step('Open select', async () => {
selectButton.focus();
await userEvent.keyboard('{ArrowDown}');
expect(selectButton).toHaveTextContent('Tadpole');
});

await step('Navigate to option 2 with ArrowDown', async () => {
await userEvent.keyboard('{ArrowDown}');
expect(selectButton).toHaveTextContent('Pollywog');
});

await step('Navigate to option 3 with ArrowDown', async () => {
await userEvent.keyboard('{ArrowDown}');
expect(selectButton).toHaveTextContent('Frog');
});

await step('Loop back to option 1 with ArrowDown', async () => {
await userEvent.keyboard('{ArrowDown}');
expect(selectButton).toHaveTextContent('Tadpole');
});

await step('Navigate backwards with ArrowUp', async () => {
await userEvent.keyboard('{ArrowUp}');
expect(selectButton).toHaveTextContent('Frog');
});

await step('Navigate backwards with ArrowUp', async () => {
await userEvent.keyboard('{ArrowUp}');
expect(selectButton).toHaveTextContent('Pollywog');
});

await step('Navigate back to option 1 with ArrowUp', async () => {
await userEvent.keyboard('{ArrowUp}');
expect(selectButton).toHaveTextContent('Tadpole');
});
},
});

export const MouseOpenNoAutoselect = meta.story({
name: 'AutoSelect - nothing selected on Mouse open (single)',
play: async ({ canvas, args, step }) => {
Expand Down
21 changes: 16 additions & 5 deletions code/core/src/components/components/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import React, { forwardRef, useCallback, useEffect, useMemo, useRef, useState }
import { RefreshIcon } from '@storybook/icons';

import { useInteractOutside } from '@react-aria/interactions';
import { Overlay, useOverlay, useOverlayPosition } from '@react-aria/overlays';
import { Overlay, ariaHideOutside, useOverlay, useOverlayPosition } from '@react-aria/overlays';
import { useObjectRef } from '@react-aria/utils';
import { useOverlayTriggerState } from '@react-stately/overlays';
import { darken, transparentize } from 'polished';
Expand Down Expand Up @@ -142,6 +142,12 @@ const MinimalistPopover: FC<{
onInteractOutside: handleClose,
});

useEffect(() => {
if (popoverRef.current) {
return ariaHideOutside([popoverRef.current], { shouldUseInert: true });
}
}, []);

const { overlayProps: positionProps } = useOverlayPosition({
targetRef: triggerRef,
overlayRef: popoverRef,
Expand Down Expand Up @@ -325,8 +331,10 @@ export const Select = forwardRef<HTMLButtonElement, SelectProps>(
return;
}

const currentIndex = options.findIndex(
(option) => externalToValue(option.value) === activeOption.value
const currentIndex = options.findIndex((option) =>
activeOption.type === 'reset'
? 'type' in option && option.type === 'reset'
: externalToValue(option.value) === activeOption.value
);
const nextIndex = currentIndex + step;

Expand All @@ -349,8 +357,11 @@ export const Select = forwardRef<HTMLButtonElement, SelectProps>(
setActiveOption(options[Math.max(0, options.length - step)]);
return;
}
const currentIndex = options.findIndex(
(option) => externalToValue(option.value) === activeOption.value

const currentIndex = options.findIndex((option) =>
activeOption.type === 'reset'
? 'type' in option && option.type === 'reset'
: externalToValue(option.value) === activeOption.value
);
const nextIndex = currentIndex - step;

Expand Down
3 changes: 3 additions & 0 deletions code/core/src/core-events/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ enum events {
// Open a file in the code editor
OPEN_IN_EDITOR_REQUEST = 'openInEditorRequest',
OPEN_IN_EDITOR_RESPONSE = 'openInEditorResponse',
// Emitted when the manager UI sets up a focus trap
MANAGER_INERT_ATTRIBUTE_CHANGED = 'managerInertAttributeChanged',
}

// Enables: `import Events from ...`
Expand Down Expand Up @@ -159,6 +161,7 @@ export const {
ARGTYPES_INFO_RESPONSE,
OPEN_IN_EDITOR_REQUEST,
OPEN_IN_EDITOR_RESPONSE,
MANAGER_INERT_ATTRIBUTE_CHANGED,
} = events;

export * from './data/create-new-story';
Expand Down
32 changes: 32 additions & 0 deletions code/core/src/manager/App.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { ComponentProps } from 'react';
import React, { useEffect } from 'react';

import Events from 'storybook/internal/core-events';
import type { Addon_PageType } from 'storybook/internal/types';

import { addons } from 'storybook/manager-api';
Expand All @@ -22,11 +23,42 @@ type Props = {
export const App = ({ managerLayoutState, setManagerLayoutState, pages, hasTab }: Props) => {
const { setMobileAboutOpen } = useLayout();

/**
* Lets us tell the UI whether or not keyboard shortcuts are enabled, in places where it's not
* convenient to load the addons singleton to figure it out.
*/
const { enableShortcuts = true } = addons.getConfig();
useEffect(() => {
document.body.setAttribute('data-shortcuts-enabled', enableShortcuts ? 'true' : 'false');
}, [enableShortcuts]);

/**
* Detects when our component library has enabled a focus trap. By convention, react-aria sets the
* document root to `inert` when a focus trap is enabled. We observe that attribute and inform the
* preview iframe when to respect the focus trap, via a channel event. This is necessary because
* inert is no longer propagated into iframes as per https://github.com/whatwg/html/issues/7605,
* and the replacement permission policy is not yet widely available
* (https://github.com/w3c/webappsec-permissions-policy/issues/273).
*/
useEffect(() => {
const rootElement = document.getElementById('root');
if (!rootElement) {
return;
}

const observer = new MutationObserver(() => {
const hasInert = rootElement.hasAttribute('inert');
addons.getChannel().emit(Events.MANAGER_INERT_ATTRIBUTE_CHANGED, hasInert);
});

observer.observe(rootElement, {
attributes: true,
attributeFilter: ['inert'],
});

return () => observer.disconnect();
}, []);

return (
<>
<Global styles={createGlobal} />
Expand Down
1 change: 1 addition & 0 deletions code/core/src/manager/globals/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,7 @@ export default {
'FORCE_REMOUNT',
'FORCE_RE_RENDER',
'GLOBALS_UPDATED',
'MANAGER_INERT_ATTRIBUTE_CHANGED',
'NAVIGATE_URL',
'OPEN_IN_EDITOR_REQUEST',
'OPEN_IN_EDITOR_RESPONSE',
Expand Down
10 changes: 8 additions & 2 deletions code/core/src/manager/settings/shortcuts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,14 @@ class ShortcutsScreen extends Component<ShortcutsScreenProps, ShortcutsScreenSta

renderKeyInput = () => {
const { shortcutKeys, addonsShortcutLabels } = this.state;
// @ts-expect-error (non strict)
const arr = Object.entries(shortcutKeys).map(([feature, { shortcut }]: [Feature, any]) => (
// Filter out keyboard shortcuts from localStorage that no longer exist in code
const availableShortcuts = (Object.entries(shortcutKeys) as [Feature, any][]).filter(
([feature]: [Feature, any]) =>
shortcutLabels[feature] !== undefined ||
(addonsShortcutLabels && addonsShortcutLabels[feature])
);

const arr = availableShortcuts.map(([feature, { shortcut }]: [Feature, any]) => (
<Row key={feature}>
{/* @ts-expect-error (non strict) */}
<Description>{shortcutLabels[feature] || addonsShortcutLabels[feature]}</Description>
Expand Down
20 changes: 19 additions & 1 deletion code/core/src/preview/runtime.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { TELEMETRY_ERROR } from 'storybook/internal/core-events';
import { MANAGER_INERT_ATTRIBUTE_CHANGED, TELEMETRY_ERROR } from 'storybook/internal/core-events';

import { global } from '@storybook/global';

Expand Down Expand Up @@ -31,6 +31,24 @@ export function setup() {
channel.emit(TELEMETRY_ERROR, prepareForTelemetry(error));
};

/**
* Ensure we synchronise the preview runtime's inert state with the manager's. The inert attribute
* used to be propagated into iframes, but this has changed, breaking focus trap implementations
* that rely on inert on the document root. We synchronise inert to ensure end user components
* don't programmatically focus when a focus trap is active in the manager UI. Otherwise, the UI
* could reach a deadlock state and be unusable.
*/
document.addEventListener('DOMContentLoaded', () => {
const channel = global.__STORYBOOK_ADDONS_CHANNEL__;
channel.on(MANAGER_INERT_ATTRIBUTE_CHANGED, (isInert: boolean) => {
if (isInert) {
document.body.setAttribute('inert', 'true');
} else {
document.body.removeAttribute('inert');
}
});
});

// handle all uncaught StorybookError at the root of the application and log to telemetry if applicable
global.addEventListener('error', errorListener);
global.addEventListener('unhandledrejection', unhandledRejectionListener);
Expand Down
75 changes: 72 additions & 3 deletions code/core/src/telemetry/event-cache.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import { describe, expect, it } from 'vitest';
import type { MockInstance } from 'vitest';
import { beforeEach, describe, expect, it, vi } from 'vitest';

import { getPrecedingUpgrade } from './event-cache';
import { cache } from 'storybook/internal/common';

import { get, getLastEvents, getPrecedingUpgrade, set } from './event-cache';

vi.mock('storybook/internal/common', { spy: true });

expect.addSnapshotSerializer({
print: (val: any) => JSON.stringify(val, null, 2),
print: (val: unknown) => JSON.stringify(val, null, 2),
test: (val) => typeof val !== 'string',
});

Expand Down Expand Up @@ -174,4 +179,68 @@ describe('event-cache', () => {
`);
});
});

describe('race condition prevention', () => {
let cacheGetMock: MockInstance;
let cacheSetMock: MockInstance;

beforeEach(() => {
vi.clearAllMocks();
cacheGetMock = vi.mocked(cache.get);
cacheSetMock = vi.mocked(cache.set);
});

it('getLastEvents waits for pending set operations to complete', async () => {
const initialData = {
init: { timestamp: 1, body: { eventType: 'init', eventId: 'init-1' } },
};
const updatedData = {
init: { timestamp: 1, body: { eventType: 'init', eventId: 'init-1' } },
upgrade: { timestamp: 2, body: { eventType: 'upgrade', eventId: 'upgrade-1' } },
};

// Use a simple delay to simulate async operations
let setGetResolved = false;
let setSetResolved = false;

cacheGetMock.mockImplementationOnce(async () => {
while (!setGetResolved) {
await new Promise((resolve) => setTimeout(resolve, 10));
}
return initialData;
});

cacheSetMock.mockImplementationOnce(async () => {
while (!setSetResolved) {
await new Promise((resolve) => setTimeout(resolve, 10));
}
});

// Mock cache.get to return updated data after set completes
cacheGetMock.mockResolvedValueOnce(updatedData);

// Start a set operation (this will be pending)
const setPromiseResult = set('upgrade', { eventType: 'upgrade', eventId: 'upgrade-1' });

// Immediately call getLastEvents() - it should wait for set() to complete
const getPromise = getLastEvents();

// Verify that getLastEvents hasn't resolved yet (it's waiting)
await new Promise((resolve) => setTimeout(resolve, 50));

// Resolve the set operations
setGetResolved = true;
await new Promise((resolve) => setTimeout(resolve, 50));
setSetResolved = true;
await setPromiseResult;

// Now getLastEvents should complete and return the updated data
const result = await getPromise;

// Verify that getLastEvents waited for set to complete and got the updated data
expect(result).toEqual(updatedData);
expect(cacheGetMock).toHaveBeenCalledTimes(2); // Once in setHelper, once in getLastEvents
expect(cacheSetMock).toHaveBeenCalledTimes(1);
});
});
});
4 changes: 4 additions & 0 deletions code/core/src/telemetry/event-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ export const get = async (eventType: EventType): Promise<CacheEntry | undefined>
};

export const getLastEvents = async (): Promise<Record<EventType, CacheEntry>> => {
// Wait for any pending set operations to complete before reading
// This prevents race conditions where getLastEvents() reads stale data
// while a set() operation is still in progress
await operation;
return (await cache.get('lastEvents')) || {};
};

Expand Down
3 changes: 2 additions & 1 deletion code/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -286,5 +286,6 @@
"Dependency Upgrades"
]
]
}
},
"deferredNextVersion": "10.1.6"
}
2 changes: 1 addition & 1 deletion docs/configure/integration/eslint-plugin.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ sidebar:
title: ESLint plugin
---

Storybook provides an dedicated [ESLint plugin](https://github.com/storybookjs/eslint-plugin-storybook) to help you write stories and components aligned with the latest Storybook and frontend development best practices.
Storybook provides a dedicated [ESLint plugin](https://github.com/storybookjs/storybook/tree/next/code/lib/eslint-plugin) to help you write stories and components aligned with the latest Storybook and frontend development best practices.

## Installation

Expand Down
2 changes: 1 addition & 1 deletion docs/versions/latest.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"10.1.5","info":{"plain":"- Addon-Vitest: Isolate error reasons during postinstall - [#33295](https://github.com/storybookjs/storybook/pull/33295), thanks @valentinpalkovic!\n- CLI: Fix react native template not copying in init - [#33308](https://github.com/storybookjs/storybook/pull/33308), thanks @dannyhw!\n- Docs: Support Rolldown bundler module namespace objects - [#33280](https://github.com/storybookjs/storybook/pull/33280), thanks @akornmeier!"}}
{"version":"10.1.6","info":{"plain":"- Manager: Do not display non-existing shortcuts in the settings page - [#32711](https://github.com/storybookjs/storybook/pull/32711), thanks @DKER2!\n- Preview: Enforce inert body if manager is focus-trapped - [#33186](https://github.com/storybookjs/storybook/pull/33186), thanks @Sidnioulz!\n- Telemetry: Await pending operations in getLastEvents to prevent race conditions - [#33285](https://github.com/storybookjs/storybook/pull/33285), thanks @valentinpalkovic!\n- UI: Fix keyboard navigation bug for \\\"reset\\\" option in `Select` - [#33268](https://github.com/storybookjs/storybook/pull/33268), thanks @Sidnioulz!"}}
2 changes: 1 addition & 1 deletion docs/versions/next.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"10.2.0-alpha.3","info":{"plain":"- Addon Docs: Skip `!autodocs` stories when computing primary story - [#32712](https://github.com/storybookjs/storybook/pull/32712), thanks @ia319!\n- Angular: Honor --loglevel and --logfile in dev/build - [#33212](https://github.com/storybookjs/storybook/pull/33212), thanks @valentinpalkovic!\n- CSF: Export type to prevent `type cannot be named`-errors - [#33216](https://github.com/storybookjs/storybook/pull/33216), thanks @unional!\n- Chore: Upgrade Chromatic CLI - [#33176](https://github.com/storybookjs/storybook/pull/33176), thanks @ghengeveld!\n- Core: Enhance getPrettier function to provide prettier interface - [#33260](https://github.com/storybookjs/storybook/pull/33260), thanks @valentinpalkovic!\n- Core: Fix cwd handling for negated globs - [#33241](https://github.com/storybookjs/storybook/pull/33241), thanks @ia319!\n- NextJS: Alias image to use fileURLToPath for better resolution - [#33256](https://github.com/storybookjs/storybook/pull/33256), thanks @ndelangen!\n- Nextj.js: Support top-level weight/style in next/font/local with string src - [#32998](https://github.com/storybookjs/storybook/pull/32998), thanks @Chiman2937!\n- Telemetry: Cache Storybook metadata by main config content hash - [#33247](https://github.com/storybookjs/storybook/pull/33247), thanks @valentinpalkovic!\n- TypeScript: Fix summary undefined type issue - [#32585](https://github.com/storybookjs/storybook/pull/32585), thanks @afsalshamsudeen!"}}
{"version":"10.2.0-alpha.4","info":{"plain":"- Addon-Vitest: Isolate error reasons during postinstall - [#33295](https://github.com/storybookjs/storybook/pull/33295), thanks @valentinpalkovic!\n- CLI: Fix react native template not copying in init - [#33308](https://github.com/storybookjs/storybook/pull/33308), thanks @dannyhw!\n- Core: Retry `writeFile` cache when EBUSY error occurs - [#32981](https://github.com/storybookjs/storybook/pull/32981), thanks @reduckted!\n- Docs: Support Rolldown bundler module namespace objects - [#33280](https://github.com/storybookjs/storybook/pull/33280), thanks @akornmeier!\n- SvelteKit: Align JS template with TS template - [#31451](https://github.com/storybookjs/storybook/pull/31451), thanks @brettearle!"}}
Loading