Address feedback

mongodb · Oct 2, 2024 · db057de · db057de
1 parent 557fb83
commit db057de
Show file tree

Hide file tree

Showing 16 changed files with 679 additions and 356 deletions.
diff --git a/packages/leafygreen-provider/src/PopoverContext/PopoverContext.types.ts b/packages/leafygreen-provider/src/PopoverContext/PopoverContext.types.ts
@@ -15,14 +15,20 @@ type TransitionLifecycleCallbacks = Pick<
   'onEnter' | 'onEntering' | 'onEntered' | 'onExit' | 'onExiting' | 'onExited'
 >;
 
-export const DismissMode = {
+/**
+ * Options to control how the popover element is dismissed. This should not be altered
+ * because it is intended to have parity with the web-native {@link https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/popover popover attribute}
+ * @param Auto will automatically handle dismissal on backdrop click or esc key press, ensuring only one popover is visible at a time
+ * @param Manual will require that the consumer handle dismissal manually
+ */
+const DismissMode = {
   Auto: 'auto',
   Manual: 'manual',
 } as const;
-export type DismissMode = (typeof DismissMode)[keyof typeof DismissMode];
+type DismissMode = (typeof DismissMode)[keyof typeof DismissMode];
 
 /** Local implementation of web-native `ToggleEvent` until we use typescript v5 */
-export interface ToggleEvent extends Event {
+interface ToggleEvent extends Event {
   type: 'toggle';
   newState: 'open' | 'closed';
   oldState: 'open' | 'closed';
@@ -119,7 +125,7 @@ interface RenderTopLayerProps {
   scrollContainer?: never;
 }
 
-export type PopoverRenderModeProps =
+type PopoverRenderModeProps =
   | RenderInlineProps
   | RenderPortalProps
   | RenderTopLayerProps;

diff --git a/packages/leafygreen-provider/src/PopoverContext/index.ts b/packages/leafygreen-provider/src/PopoverContext/index.ts
@@ -4,8 +4,6 @@ export {
   usePopoverContext,
 } from './PopoverContext';
 export {
-  DismissMode,
   type PopoverContextType,
   type PopoverProviderProps,
-  type ToggleEvent,
 } from './PopoverContext.types';
diff --git a/packages/leafygreen-provider/src/index.ts b/packages/leafygreen-provider/src/index.ts
@@ -8,12 +8,10 @@ export {
   useOverlayContext,
 } from './OverlayContext';
 export {
-  DismissMode,
   PopoverContext,
   type PopoverContextType,
   PopoverProvider,
   type PopoverProviderProps,
-  type ToggleEvent,
   usePopoverContext,
 } from './PopoverContext';
 export { useBaseFontSize } from './TypographyContext';

diff --git a/packages/popover/src/Popover.stories.tsx b/packages/popover/src/Popover.stories.tsx
@@ -3,20 +3,22 @@ import {
   storybookExcludedControlParams,
   StoryMetaType,
 } from '@lg-tools/storybook-utils';
-import { StoryFn } from '@storybook/react';
+import { StoryFn, StoryObj } from '@storybook/react';
 
 import Button from '@leafygreen-ui/button';
 import { css, cx } from '@leafygreen-ui/emotion';
 import { palette } from '@leafygreen-ui/palette';
 import { color } from '@leafygreen-ui/tokens';
 
-import Popover, {
+import {
   Align,
+  DismissMode,
   Justify,
+  Popover,
   PopoverProps,
   RenderMode,
   ToggleEvent,
-} from '.';
+} from './Popover';
 
 const popoverStyle = css`
   border: 1px solid ${palette.gray.light1};
@@ -84,21 +86,22 @@ const referenceElPositions: { [key: string]: string } = {
   `,
 };
 
+const defaultExcludedControls = [
+  ...storybookExcludedControlParams,
+  'active',
+  'children',
+  'portalClassName',
+  'refButtonPosition',
+  'refEl',
+];
+
 const meta: StoryMetaType<typeof Popover> = {
   title: 'Components/Popover',
   component: Popover,
   parameters: {
     default: 'LiveExample',
     controls: {
-      exclude: [
-        ...storybookExcludedControlParams,
-        'active',
-        'children',
-        'portalClassName',
-        'refButtonPosition',
-        'refEl',
-        'renderMode',
-      ],
+      exclude: defaultExcludedControls,
     },
     generate: {
       storyNames: [
@@ -118,9 +121,6 @@ const meta: StoryMetaType<typeof Popover> = {
       },
       // eslint-disable-next-line react/display-name
       decorator: Instance => {
-        // eslint-disable-next-line react-hooks/rules-of-hooks
-        const [active, setActive] = useState<boolean>(false);
-
         return (
           <div
             className={css`
@@ -132,7 +132,7 @@ const meta: StoryMetaType<typeof Popover> = {
               justify-content: center;
             `}
           >
-            <Button onClick={() => setActive(active => !active)}>
+            <Button>
               Button Text
               <Instance
                 buttonText={undefined}
@@ -151,6 +151,7 @@ const meta: StoryMetaType<typeof Popover> = {
     adjustOnMutation: false,
     align: Align.Top,
     buttonText: 'Button Text',
+    dismissMode: DismissMode.Auto,
     justify: Justify.Start,
     spacing: 4,
   },
@@ -164,6 +165,10 @@ const meta: StoryMetaType<typeof Popover> = {
       description:
         'Storybook only prop. Used to change the reference button text',
     },
+    dismissMode: {
+      options: Object.values(DismissMode),
+      control: { type: 'radio' },
+    },
     justify: {
       options: Object.values(Justify),
       control: { type: 'radio' },
@@ -183,7 +188,6 @@ type PopoverStoryProps = PopoverProps & {
   buttonText: string;
   refButtonPosition: string;
 };
-
 export const LiveExample: StoryFn<PopoverStoryProps> = ({
   refButtonPosition,
   buttonText,
@@ -202,6 +206,10 @@ export const LiveExample: StoryFn<PopoverStoryProps> = ({
 
   const position = referenceElPositions[refButtonPosition];
 
+  const handleClick = () => {
+    setActive(active => !active);
+  };
+
   const handleToggle = (e: ToggleEvent) => {
     const newActive = e.newState === 'open';
     setActive(newActive);
@@ -211,7 +219,7 @@ export const LiveExample: StoryFn<PopoverStoryProps> = ({
     <div className={containerStyles}>
       <Button
         className={cx(buttonStyles, position)}
-        onClick={() => setActive(active => !active)}
+        onClick={handleClick}
         ref={buttonRef}
       >
         {buttonText}
@@ -234,9 +242,11 @@ LiveExample.parameters = {
   },
 };
 
-export const RenderModePortalInScrollableContainer: StoryFn<
-  PopoverStoryProps
-> = ({ refButtonPosition, buttonText, ...args }: PopoverStoryProps) => {
+const PortalPopoverInScrollableContainer = ({
+  refButtonPosition,
+  buttonText,
+  ...args
+}: PopoverStoryProps) => {
   const { dismissMode, onToggle, renderMode, ...rest } = args;
   const [active, setActive] = useState<boolean>(false);
   const portalRef = useRef<HTMLElement | null>(null);
@@ -267,20 +277,26 @@ export const RenderModePortalInScrollableContainer: StoryFn<
     </div>
   );
 };
-RenderModePortalInScrollableContainer.parameters = {
-  chromatic: {
-    disableSnapshot: true,
+export const RenderModePortalInScrollableContainer = {
+  render: PortalPopoverInScrollableContainer,
+  parameters: {
+    chromatic: {
+      disableSnapshot: true,
+    },
+    controls: {
+      exclude: [...defaultExcludedControls, 'dismissMode', 'renderMode'],
+    },
+  },
+  argTypes: {
+    renderMode: { control: 'none' },
+    portalClassName: { control: 'none' },
+    refEl: { control: 'none' },
+    className: { control: 'none' },
+    active: { control: 'none' },
   },
-};
-RenderModePortalInScrollableContainer.argTypes = {
-  renderMode: { control: 'none' },
-  portalClassName: { control: 'none' },
-  refEl: { control: 'none' },
-  className: { control: 'none' },
-  active: { control: 'none' },
 };
 
-export const RenderModeInline: StoryFn<PopoverStoryProps> = ({
+const InlinePopover = ({
   refButtonPosition,
   buttonText,
   ...args
@@ -315,57 +331,110 @@ export const RenderModeInline: StoryFn<PopoverStoryProps> = ({
     </div>
   );
 };
-RenderModeInline.parameters = {
-  chromatic: {
-    disableSnapshot: true,
+export const RenderModeInline = {
+  render: InlinePopover,
+  parameters: {
+    chromatic: {
+      disableSnapshot: true,
+    },
+    controls: {
+      exclude: [...defaultExcludedControls, 'dismissMode', 'renderMode'],
+    },
+  },
+  argTypes: {
+    renderMode: { control: 'none' },
+    portalClassName: { control: 'none' },
+    refEl: { control: 'none' },
+    className: { control: 'none' },
+    active: { control: 'none' },
   },
 };
-RenderModeInline.argTypes = {
-  renderMode: { control: 'none' },
-  portalClassName: { control: 'none' },
-  refEl: { control: 'none' },
-  className: { control: 'none' },
-  active: { control: 'none' },
-};
+
+const generatedStoryExcludedControlParams = [
+  ...storybookExcludedControlParams,
+  'active',
+  'adjustOnMutation',
+  'align',
+  'buttonText',
+  'children',
+  'dismissMode',
+  'justify',
+  'portalClassName',
+  'refButtonPosition',
+  'refEl',
+  'renderMode',
+  'spacing',
+  'usePortal',
+];
 
 export const Top = {
-  render: () => {},
+  render: LiveExample.bind({}),
   args: {
     align: Align.Top,
   },
+  parameters: {
+    controls: {
+      exclude: generatedStoryExcludedControlParams,
+    },
+  },
 };
 
 export const Bottom = {
-  render: () => {},
+  render: LiveExample.bind({}),
   args: {
     align: Align.Bottom,
   },
+  parameters: {
+    controls: {
+      exclude: generatedStoryExcludedControlParams,
+    },
+  },
 };
 
 export const Left = {
-  render: () => {},
+  render: LiveExample.bind({}),
   args: {
     align: Align.Left,
   },
+  parameters: {
+    controls: {
+      exclude: generatedStoryExcludedControlParams,
+    },
+  },
 };
 
 export const Right = {
-  render: () => {},
+  render: LiveExample.bind({}),
   args: {
     align: Align.Right,
   },
+  parameters: {
+    controls: {
+      exclude: generatedStoryExcludedControlParams,
+    },
+  },
 };
 
 export const CenterHorizontal = {
-  render: () => {},
+  render: LiveExample.bind({}),
   args: {
     align: Align.CenterHorizontal,
   },
+  parameters: {
+    controls: {
+      exclude: generatedStoryExcludedControlParams,
+    },
+  },
 };
 
-export const CenterVertical = {
-  render: () => {},
+export const CenterVertical: StoryObj<PopoverStoryProps> = {
+  render: LiveExample.bind({}),
   args: {
     align: Align.CenterVertical,
   },
+  parameters: {
+    controls: {
+      exclude: generatedStoryExcludedControlParams,
+    },
+  },
 };