WIP: Revert breaking changes on Button, Tabs, Modal

Author: Sidnioulz

MIGRATION.md

@@ -13,7 +13,7 @@
       - [Added: ariaLabel](#added-arialabel)
       - [Added: shortcut](#added-shortcut)
       - [Added: tooltip](#added-tooltip)
-      - [Removed: active](#removed-active)
+      - [Deprecated: active](#deprecated-active)
     - [IconButton is deprecated](#iconbutton-is-deprecated)
     - [Bar Component API Changes](#bar-component-api-changes)
       - [Added: innerStyle](#added-innerstyle)
@@ -22,7 +22,7 @@
     - [TabsState is deprecated](#tabsstate-is-deprecated)
     - [TabWrapper is deprecated](#tabwrapper-is-deprecated)
     - [TabButton is deprecated](#tabbutton-is-deprecated)
-    - [TabBar is removed](#tabbar-is-removed)
+    - [TabBar is deprecated](#tabbar-is-deprecated)
     - [Modal Component API Changes](#modal-component-api-changes)
       - [Removed: container and portalSelector](#removed-container-and-portalselector)
       - [Removed: onInteractOutside](#removed-oninteractoutside)
@@ -629,7 +629,7 @@ During development of Storybook [Tags](https://storybook.js.org/docs/writing-sto
 #### Button Component API Changes
 
 ##### Added: ariaLabel
-The Button component now has a mandatory `ariaLabel` prop, to ensure that Storybook UI code is accessible to screenreader users.
+The Button component now has an `ariaLabel` prop, to ensure that Storybook UI code is accessible to screenreader users. The prop will become mandatory in Storybook 11.
 
 When buttons have text content as children, and when that text content does not rely on visual context to be understood, you may pass `false` to the `ariaLabel` prop to indicate that an ARIA label is not necessary.
 
@@ -643,13 +643,11 @@ An optional `shortcut` prop was added for internal use. When `shortcut` is set,
 
 Button now displays a tooltip whenever `ariaLabel` or `shortcut` is set. The tooltip can be customised by passing a string to the optional `tooltip` prop.
 
-##### Removed: active
+##### Deprecated: active
 
-The `active` prop was removed from Button.
+The `active` prop is deprecated and will be removed in Storybook 11.
 
-The Button component has historically been used to implement Toggle and Select interactions. When you need a Button to have an active state, use ToggleButton if the active state denotes that a state or feature is enabled after pressing the Button.
-
-Use Select if the active state denotes that the Button is open while a selection is being made, or that the Button currently has a selected value.
+The Button component has historically been used to implement Toggle and Select interactions. When you need a Button to have an active state, use ToggleButton if the active state denotes that a state or feature is enabled after pressing the Button. Use Select if the active state denotes that the Button is open while a selection is being made, or that the Button currently has a selected value.
 
 #### IconButton is deprecated
 
@@ -695,9 +693,9 @@ The `TabWrapper` component is deprecated as it was not accessible. Instead, use
 
 The `TabButton` class is deprecated as it was not accessible. It does not have a replacement, as the new `TabList` component handles tab buttons internally.
 
-#### TabBar is removed
+#### TabBar is deprecated
 
-The `TabBar` component, a styled bar used inside `Tabs` and not intended to be public, has been removed. It has no replacement.
+The `TabBar` component, a styled bar used inside `Tabs` and not intended to be public, is deprecated and will be hidden in Storybook 11. Use `TabsView` instead.
 
 #### Modal Component API Changes
 
@@ -711,10 +709,10 @@ The `onInteractOutside` prop is removed in favor of `dismissOnClickOutside`, bec
 The `onEscapeKeyDown` prop is removed in favor of `dismissOnEscape`, because it was only used to close the modal when pressing Escape. Use `dismissOnEscape` to control whether pressing Escape should close it or not.
 
 ##### Added: `ariaLabel`
-Modal elements must have a title to be accessible. Set that title through the mandatory `ariaLabel` prop.
+Modal elements must have a title to be accessible. Set that title through the `ariaLabel` prop. It will become mandatory in Storybook 11.
 
 ##### Renamed: Modal.Dialog.Close and Modal.CloseButton
-The `Modal.Dialog.Close` component and `Modal.CloseButton` components are replaced by `Modal.Close` for consistency with other components. Those names are deprecated and will be removed in a future version. You may call `<Modal.Close />` for a default close button, or `<Modal.Close asChild>...</Modal.Close>` to wrap your own custom button.
+The `Modal.Dialog.Close` component and `Modal.CloseButton` components are replaced by `Modal.Close` for consistency with other components. Those names are deprecated and will be removed in Storybook 11. You may call `<Modal.Close />` for a default close button, or `<Modal.Close asChild>...</Modal.Close>` to wrap your own custom button.
 
 The `Modal.Close` component no longer requires an `onClick` handler to close the modal. It will automatically close the modal when clicked. If you need to perform additional actions when the close button is clicked, you can still provide an `onClick` handler, and it will be called in addition to closing the modal.
 #### ListItem, TooltipLinkList and TooltipMessage are deprecated

code/core/src/components/components/Button/Button.stories.tsx

@@ -101,13 +101,13 @@ export const PseudoStates = meta.story({
       </Row>
       <Row id="active">
         <Button ariaLabel={false} variant="solid">
-          active
+          Active
         </Button>
         <Button ariaLabel={false} variant="outline">
-          active
+          Active
         </Button>
         <Button ariaLabel={false} variant="ghost">
-          active
+          Active
         </Button>
       </Row>
       <Row id="focus">
@@ -144,6 +144,78 @@ export const PseudoStates = meta.story({
   },
 });
 
+export const Active = meta.story({
+  name: 'Active (deprecated)',
+  args: { ariaLabel: false, active: true },
+  render: (args) => (
+    <Stack>
+      <Row>
+        <Button {...args} variant="solid">
+          Button
+        </Button>
+        <Button {...args} variant="outline">
+          Button
+        </Button>
+        <Button {...args} variant="ghost">
+          Button
+        </Button>
+      </Row>
+      <Row id="hover">
+        <Button {...args} variant="solid">
+          Hover
+        </Button>
+        <Button {...args} variant="outline">
+          Hover
+        </Button>
+        <Button {...args} variant="ghost">
+          Hover
+        </Button>
+      </Row>
+      <Row id="active">
+        <Button {...args} variant="solid">
+          active
+        </Button>
+        <Button {...args} variant="outline">
+          active
+        </Button>
+        <Button {...args} variant="ghost">
+          active
+        </Button>
+      </Row>
+      <Row id="focus">
+        <Button {...args} variant="solid">
+          Focus
+        </Button>
+        <Button {...args} variant="outline">
+          Focus
+        </Button>
+        <Button {...args} variant="ghost">
+          Focus
+        </Button>
+      </Row>
+      <Row id="focus-visible">
+        <Button {...args} variant="solid">
+          Focus Visible
+        </Button>
+        <Button {...args} variant="outline">
+          Focus Visible
+        </Button>
+        <Button {...args} variant="ghost">
+          Focus Visible
+        </Button>
+      </Row>
+    </Stack>
+  ),
+  parameters: {
+    pseudo: {
+      hover: '#hover button',
+      active: '#active button',
+      focus: '#focus button',
+      focusVisible: '#focus-visible button',
+    },
+  },
+});
+
 export const WithIcon = meta.story({
   args: {
     ariaLabel: false,

code/core/src/components/components/Button/Button.tsx

@@ -1,7 +1,7 @@
 import type { ButtonHTMLAttributes, SyntheticEvent } from 'react';
 import React, { forwardRef, useEffect, useMemo, useState } from 'react';
 
-import { deprecate } from 'storybook/internal/client-logger';
+import { deprecate, logger } from 'storybook/internal/client-logger';
 
 import { Slot } from '@radix-ui/react-slot';
 import { darken, lighten, rgba, transparentize } from 'polished';
@@ -17,6 +17,7 @@ export interface ButtonProps extends ButtonHTMLAttributes<HTMLButtonElement> {
   padding?: 'small' | 'medium' | 'none';
   variant?: 'outline' | 'solid' | 'ghost';
   onClick?: (event: SyntheticEvent) => void;
+  active?: boolean;
   disabled?: boolean;
   animation?: 'none' | 'rotate360' | 'glow' | 'jiggle';
 
@@ -26,7 +27,7 @@ export interface ButtonProps extends ButtonHTMLAttributes<HTMLButtonElement> {
    * Button's content is already accessible to all. When a string is passed, it is also used as the
    * default tooltip text.
    */
-  ariaLabel: string | false;
+  ariaLabel?: string | false;
 
   /**
    * An optional tooltip to display when the Button is hovered. If the Button has no text content,
@@ -64,6 +65,7 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
       variant = 'outline',
       padding = 'medium',
       disabled = false,
+      active,
       onClick,
       ariaLabel,
       ariaDescription = undefined,
@@ -76,17 +78,29 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
   ) => {
     let Comp: 'button' | 'a' | typeof Slot = 'button';
 
+    // FIXME / TODO : reenable this but its too verbose and i cant work rn
+    // if (ariaLabel === undefined || ariaLabel === '') {
+    //   logger.warn(ariaLabel, props.children);
+    //   logger.warn(
+    //     'The `ariaLabel` prop on `Button` will become mandatory in Storybook 11. Buttons with text content should set `ariaLabel={false}` to indicate that they are accessible as-is. Buttons without text content must provide a meaningful `ariaLabel` for accessibility.'
+    //   );
+    //   // TODO in Storybook 11
+    //   // throw new Error(
+    //   //   'Button requires an ARIA label to be accessible. Please provide a valid ariaLabel prop.'
+    //   // );
+    // }
+
+    if (active !== undefined) {
+      deprecate(
+        'The `active` prop on `Button` is deprecated and will be removed in Storybook 11. Use specialized components like `ToggleButton` or `Select` instead.'
+      );
+    }
+
     if (asChild) {
       Comp = Slot;
     }
     const { ariaDescriptionAttrs, AriaDescription } = useAriaDescription(ariaDescription);
 
-    if (ariaLabel === '') {
-      throw new Error(
-        'Button requires an ARIA label to be accessible. Please provide a valid ariaLabel prop.'
-      );
-    }
-
     const shortcutAttribute = useMemo(() => {
       return shortcut ? shortcutToAriaKeyshortcuts(shortcut) : undefined;
     }, [shortcut]);
@@ -129,6 +143,7 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
             size={size}
             padding={padding}
             disabled={disabled}
+            active={active}
             animating={isAnimating}
             animation={animation}
             onClick={handleClick}
@@ -153,7 +168,7 @@ const StyledButton = styled('button', {
     animating: boolean;
     animation: ButtonProps['animation'];
   }
->(({ theme, variant, size, disabled, animating, animation = 'none', padding }) => ({
+>(({ theme, variant, size, disabled, active, animating, animation = 'none', padding }) => ({
   border: 0,
   cursor: disabled ? 'not-allowed' : 'pointer',
   display: 'inline-flex',
@@ -203,6 +218,10 @@ const StyledButton = styled('button', {
       return theme.button.background;
     }
 
+    if (variant === 'ghost' && active) {
+      return transparentize(0.93, theme.barSelectedColor);
+    }
+
     return 'transparent';
   })(),
   color: (() => {
@@ -214,6 +233,10 @@ const StyledButton = styled('button', {
       return theme.input.color;
     }
 
+    if (variant === 'ghost' && active) {
+      return theme.base === 'light' ? darken(0.1, theme.color.secondary) : theme.color.secondary;
+    }
+
     if (variant === 'ghost') {
       return theme.textMutedColor;
     }

code/core/src/components/components/Modal/Modal.stories.tsx

@@ -3,7 +3,7 @@ import React, { useState } from 'react';
 import { Button } from 'storybook/internal/components';
 
 import { action } from 'storybook/actions';
-import { expect, fn, getByRole, screen, userEvent, waitFor, within } from 'storybook/test';
+import { expect, fn, screen, userEvent, waitFor, within } from 'storybook/test';
 
 import preview from '../../../../../.storybook/preview';
 import { Modal } from './Modal';
@@ -221,6 +221,91 @@ export const DismissalBehavior = meta.story({
   ),
 });
 
+export const OnInteractOutside = meta.story({
+  name: 'OnInteractOutside (deprecated)',
+  args: {
+    children: <SampleModalContent />,
+    onInteractOutside: fn(),
+  },
+  render: (args) => {
+    const [isOpen, setOpen] = useState(false);
+
+    return (
+      <>
+        <Modal {...args} open={isOpen} onOpenChange={setOpen} />
+        <Button ariaLabel={false} onClick={() => setOpen(true)}>
+          Open Modal
+        </Button>
+        <Button ariaLabel={false} style={{ marginLeft: '1rem' }}>
+          Outside Button
+        </Button>
+      </>
+    );
+  },
+  play: async ({ args, canvas, step }) => {
+    await step('Open modal', async () => {
+      const trigger = canvas.getByText('Open Modal');
+      await userEvent.click(trigger);
+      await waitFor(() => {
+        expect(screen.queryByText('Sample Modal')).toBeInTheDocument();
+      });
+    });
+
+    await step('Click outside to close', async () => {
+      const outsideButton = canvas.getByText('Outside Button');
+      await userEvent.click(outsideButton);
+      expect(args.onInteractOutside).toHaveBeenCalled();
+      await waitFor(() => {
+        expect(screen.queryByText('Sample Modal')).not.toBeInTheDocument();
+      });
+    });
+  },
+});
+
+export const OnInteractOutsidePreventDefault = meta.story({
+  name: 'OnInteractOutside - e.preventDefault (deprecated)',
+  args: {
+    children: <SampleModalContent />,
+    onInteractOutside: (e) => e.preventDefault(),
+  },
+  render: (args) => {
+    const [isOpen, setOpen] = useState(false);
+
+    return (
+      <>
+        <Modal {...args} open={isOpen} onOpenChange={setOpen} />
+        <Button ariaLabel={false} onClick={() => setOpen(true)}>
+          Open Modal
+        </Button>
+        <Button ariaLabel={false} style={{ marginLeft: '1rem' }}>
+          Outside Button
+        </Button>
+      </>
+    );
+  },
+  play: async ({ canvas, step }) => {
+    await step('Open modal', async () => {
+      const trigger = canvas.getByText('Open Modal');
+      await userEvent.click(trigger);
+      await waitFor(() => {
+        expect(screen.queryByText('Sample Modal')).toBeInTheDocument();
+      });
+    });
+
+    await step('Click outside to close but modal stays open', async () => {
+      const outsideButton = canvas.getByText('Outside Button');
+      await userEvent.click(outsideButton);
+      // Wait a bit to ensure the modal close animation would've had time to play.
+      await new Promise((r) => setTimeout(r, 300));
+      await waitFor(() => {
+        expect(screen.queryByText('Sample Modal')).toBeInTheDocument();
+      });
+    });
+  },
+});
+
+// TODO what when dismissOutside is disabled
+
 const ModalWithTrigger = ({
   triggerText,
   ...modalProps