experimental/SelectPanel: Cancel + close panel when user clicks outsi…

…de (#4294) * close on clicking outside * add link to decision log * Create nervous-dogs-change.md * add onCancel to all examples * oopsie!
primer · Feb 22, 2024 · 5ca5c0a · 5ca5c0a
1 parent 54ed0a8
commit 5ca5c0a
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 35 deletions.
diff --git a/.changeset/nervous-dogs-change.md b/.changeset/nervous-dogs-change.md
@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+experimental/SelectPanel: Cancel + close panel when user clicks outside
diff --git a/packages/react/src/drafts/SelectPanel2/SelectPanel.examples.stories.tsx b/packages/react/src/drafts/SelectPanel2/SelectPanel.examples.stories.tsx
@@ -29,6 +29,9 @@ export const Minimal = () => {
     // eslint-disable-next-line no-console
     console.log('form submitted')
   }
+  const onCancel = () => {
+    setSelectedLabelIds(initialSelectedLabels)
+  }
 
   const sortingFn = (itemA: {id: string}, itemB: {id: string}) => {
     const initialSelectedIds = data.issue.labelIds
@@ -44,7 +47,7 @@ export const Minimal = () => {
     <>
       <h1>Minimal SelectPanel</h1>
 
-      <SelectPanel title="Select labels" onSubmit={onSubmit}>
+      <SelectPanel title="Select labels" onSubmit={onSubmit} onCancel={onCancel}>
         <SelectPanel.Button>Assign label</SelectPanel.Button>
 
         <ActionList>
@@ -80,6 +83,9 @@ export const WithGroups = () => {
   const onSubmit = () => {
     data.issue.assigneeIds = selectedAssigneeIds // pretending to persist changes
   }
+  const onCancel = () => {
+    setSelectedAssigneeIds(initialAssigneeIds)
+  }
 
   /* Filtering */
   const [filteredUsers, setFilteredUsers] = React.useState(data.collaborators)
@@ -120,7 +126,12 @@ export const WithGroups = () => {
     <>
       <h1>SelectPanel with groups</h1>
 
-      <SelectPanel title="Request up to 100 reviewers" onSubmit={onSubmit} onClearSelection={onClearSelection}>
+      <SelectPanel
+        title="Request up to 100 reviewers"
+        onSubmit={onSubmit}
+        onCancel={onCancel}
+        onClearSelection={onClearSelection}
+      >
         <SelectPanel.Button
           variant="invisible"
           trailingAction={GearIcon}
@@ -194,7 +205,12 @@ export const AsyncWithSuspendedList = () => {
   return (
     <>
       <h1>Async: Suspended list</h1>
-      <p>Fetching items once when the panel is opened (like repo labels)</p>
+      <p>
+        Fetching items once when the panel is opened (like repo labels)
+        <br />
+        Note: Save and Cancel is not implemented in this demo
+      </p>
+
       <SelectPanel title="Select labels">
         <SelectPanel.Button>Assign label</SelectPanel.Button>
 
@@ -344,13 +360,16 @@ export const AsyncSearchWithUseTransition = () => {
     // eslint-disable-next-line no-console
     console.log('form submitted')
   }
+  const onCancel = () => {
+    setSelectedUserIds(initialAssigneeIds)
+  }
 
   return (
     <>
       <h1>Async: search with useTransition</h1>
       <p>Fetching items on every keystroke search (like github users)</p>
 
-      <SelectPanel title="Select collaborators" onSubmit={onSubmit}>
+      <SelectPanel title="Select collaborators" onSubmit={onSubmit} onCancel={onCancel}>
         <SelectPanel.Button>Select assignees</SelectPanel.Button>
         <SelectPanel.Header>
           <SelectPanel.SearchInput loading={isPending} onChange={onSearchInputChange} />
@@ -481,6 +500,9 @@ export const WithFilterButtons = () => {
     // eslint-disable-next-line no-console
     console.log('form submitted')
   }
+  const onCancel = () => {
+    setSelectedRef(savedInitialRef)
+  }
 
   /* Filter */
   const [query, setQuery] = React.useState('')
@@ -522,9 +544,9 @@ export const WithFilterButtons = () => {
 
   return (
     <>
-      <h1>With Filter Buttons</h1>
+      <h1>With Filter Buttons {savedInitialRef}</h1>
 
-      <SelectPanel title="Switch branches/tags" onSubmit={onSubmit}>
+      <SelectPanel title="Switch branches/tags" onSubmit={onSubmit} onCancel={onCancel}>
         <SelectPanel.Button leadingVisual={GitBranchIcon} trailingVisual={TriangleDownIcon}>
           {savedInitialRef}
         </SelectPanel.Button>
@@ -582,13 +604,17 @@ export const WithFilterButtons = () => {
 }
 
 export const ShortSelectPanel = () => {
-  const [channels, setChannels] = React.useState({GitHub: false, Email: false})
+  const initialChannels = {GitHub: false, Email: false}
+  const [channels, setChannels] = React.useState(initialChannels)
   const [onlyFailures, setOnlyFailures] = React.useState(false)
 
   const onSubmit = () => {
     // eslint-disable-next-line no-console
     console.log('form submitted')
   }
+  const onCancel = () => {
+    setChannels(initialChannels)
+  }
 
   const toggleChannel = (channel: keyof typeof channels) => {
     setChannels({...channels, [channel]: !channels[channel]})
@@ -602,7 +628,7 @@ export const ShortSelectPanel = () => {
       <p>
         Use <code>height=fit-content</code> to match height of contents
       </p>
-      <SelectPanel title="Select notification channels" onSubmit={onSubmit}>
+      <SelectPanel title="Select notification channels" onSubmit={onSubmit} onCancel={onCancel}>
         <SelectPanel.Button>
           <Text sx={{color: 'fg.muted'}}>Notify me:</Text>{' '}
           {Object.keys(channels)

diff --git a/packages/react/src/drafts/SelectPanel2/SelectPanel.features.stories.tsx b/packages/react/src/drafts/SelectPanel2/SelectPanel.features.stories.tsx
@@ -63,6 +63,9 @@ export const WithWarning = () => {
   const onSubmit = () => {
     data.issue.assigneeIds = selectedAssigneeIds // pretending to persist changes
   }
+  const onCancel = () => {
+    setSelectedAssigneeIds(initialAssigneeIds)
+  }
 
   /* Filtering */
   const [filteredUsers, setFilteredUsers] = React.useState(data.collaborators)
@@ -107,6 +110,7 @@ export const WithWarning = () => {
         title="Set assignees"
         description={`Select up to ${MAX_LIMIT} people`}
         onSubmit={onSubmit}
+        onCancel={onCancel}
         onClearSelection={onClearSelection}
       >
         <SelectPanel.Button
@@ -173,6 +177,9 @@ export const WithErrors = () => {
   const onSubmit = () => {
     data.issue.assigneeIds = selectedAssigneeIds // pretending to persist changes
   }
+  const onCancel = () => {
+    setSelectedAssigneeIds(initialAssigneeIds)
+  }
 
   /* Filtering */
   const [filteredUsers, setFilteredUsers] = React.useState(
@@ -254,7 +261,7 @@ export const WithErrors = () => {
         />
       </Box>
 
-      <SelectPanel title="Set assignees" onSubmit={onSubmit} onClearSelection={onClearSelection}>
+      <SelectPanel title="Set assignees" onSubmit={onSubmit} onCancel={onCancel} onClearSelection={onClearSelection}>
         <SelectPanel.Button
           variant="invisible"
           trailingAction={GearIcon}
@@ -325,6 +332,10 @@ export const ExternalAnchor = () => {
     console.log('form submitted')
   }
 
+  const onCancel = () => {
+    setSelectedLabelIds(initialSelectedLabels)
+  }
+
   const sortingFn = (itemA: {id: string}, itemB: {id: string}) => {
     const initialSelectedIds = data.issue.labelIds
     if (initialSelectedIds.includes(itemA.id) && initialSelectedIds.includes(itemB.id)) return 1
@@ -364,7 +375,10 @@ export const ExternalAnchor = () => {
           setOpen(false) // close on submit
           onSubmit()
         }}
-        onCancel={() => setOpen(false)} // close on cancel
+        onCancel={() => {
+          onCancel()
+          setOpen(false) // close on cancel
+        }}
       >
         <ActionList>
           {itemsToShow.map(label => (
@@ -402,6 +416,10 @@ export const AsModal = () => {
     console.log('form submitted')
   }
 
+  const onCancel = () => {
+    setSelectedLabelIds(initialSelectedLabels)
+  }
+
   const sortingFn = (itemA: {id: string}, itemB: {id: string}) => {
     const initialSelectedIds = data.issue.labelIds
     if (initialSelectedIds.includes(itemA.id) && initialSelectedIds.includes(itemB.id)) return 1
@@ -416,7 +434,7 @@ export const AsModal = () => {
     <>
       <h1>SelectPanel as Modal</h1>
 
-      <SelectPanel variant="modal" title="Select labels" onSubmit={onSubmit}>
+      <SelectPanel variant="modal" title="Select labels" onSubmit={onSubmit} onCancel={onCancel}>
         <SelectPanel.Button>Assign label</SelectPanel.Button>
 
         <ActionList>

diff --git a/packages/react/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/packages/react/src/drafts/SelectPanel2/SelectPanel.stories.tsx
@@ -25,6 +25,10 @@ export const Default = () => {
     data.issue.labelIds = selectedLabelIds // pretending to persist changes
   }
 
+  const onCancel = () => {
+    setSelectedLabelIds(initialSelectedLabels)
+  }
+
   /* Filtering */
   const [filteredLabels, setFilteredLabels] = React.useState(data.labels)
   const [query, setQuery] = React.useState('')
@@ -61,16 +65,7 @@ export const Default = () => {
 
   return (
     <>
-      <SelectPanel
-        title="Select labels"
-        onSubmit={onSubmit}
-        onCancel={() => {
-          /* optional callback, for example: for multi-step overlay or to fire sync actions */
-          // eslint-disable-next-line no-console
-          console.log('panel was closed')
-        }}
-        onClearSelection={onClearSelection}
-      >
+      <SelectPanel title="Select labels" onSubmit={onSubmit} onCancel={onCancel} onClearSelection={onClearSelection}>
         <SelectPanel.Button>Assign label</SelectPanel.Button>
 
         <SelectPanel.Header>

diff --git a/packages/react/src/drafts/SelectPanel2/SelectPanel.tsx b/packages/react/src/drafts/SelectPanel2/SelectPanel.tsx
@@ -207,17 +207,10 @@ const Panel: React.FC<SelectPanelProps> = ({
   )
 
   /* 
-    We don't close the panel when clicking outside.
-    For many years, we used to save changes and closed the dialog (for label picker)
-    which isn't accessible, clicking outside should discard changes and close the dialog
-    Fixing this a11y bug would confuse users, so as a middle ground,
-    we don't close the menu and nudge the user towards the footer actions
+    We want to cancel and close the panel when user clicks outside.
+    See decision log: https://github.com/github/primer/discussions/2614#discussioncomment-8544561
   */
-  const [footerAnimationEnabled, setFooterAnimationEnabled] = React.useState(false)
-  const onClickOutside = () => {
-    setFooterAnimationEnabled(true)
-    window.setTimeout(() => setFooterAnimationEnabled(false), 350)
-  }
+  const onClickOutside = onInternalCancel
 
   return (
     <>
@@ -241,9 +234,6 @@ const Panel: React.FC<SelectPanelProps> = ({
           ...(variant === 'anchored' ? {margin: 0, top: position?.top, left: position?.left} : {}),
           '::backdrop': {backgroundColor: variant === 'anchored' ? 'transparent' : 'primer.canvas.backdrop'},
 
-          '& [data-selectpanel-primary-actions]': {
-            animation: footerAnimationEnabled ? 'selectpanel-gelatine 350ms linear' : 'none',
-          },
           '@keyframes selectpanel-gelatine': {
             '0%': {transform: 'scale(1, 1)'},
             '25%': {transform: 'scale(0.9, 1.1)'},
@@ -461,7 +451,7 @@ const SelectPanelFooter = ({...props}) => {
         <Box sx={{flexGrow: hidePrimaryActions ? 1 : 0}}>{props.children}</Box>
 
         {hidePrimaryActions ? null : (
-          <Box data-selectpanel-primary-actions sx={{display: 'flex', gap: 2}}>
+          <Box sx={{display: 'flex', gap: 2}}>
             <Button size="small" type="button" onClick={() => onCancel()}>
               Cancel
             </Button>