Fix popover closing on click inside popup content (#4739)

Author: fiskus

catalog/CHANGELOG.md

@@ -18,6 +18,7 @@ where verb is one of
 
 ## Changes
 
+- [Fixed] Toolbar popover closing on click inside popup content, preventing text selection in code samples ([#4739](https://github.com/quiltdata/quilt/pull/4739))
 - [Changed] Package creation from S3 files now includes current bucket as default destination when no workflow config exists; cross-bucket push strictly respects workflow successors configuration ([#4734](https://github.com/quiltdata/quilt/pull/4734))
 - [Fixed] Add missing `deleteObject` property to GUI config editor to enable/disable delete buttons for files and directories ([#4692](https://github.com/quiltdata/quilt/pull/4692))
 - [Changed] Hide file and directory delete buttons in Bucket tab by default; enable with `ui.actions.deleteObject` ([#4689](https://github.com/quiltdata/quilt/pull/4689))

catalog/app/components/Buttons/WithPopover.spec.tsx

@@ -3,7 +3,7 @@ import { render, fireEvent, screen, cleanup } from '@testing-library/react'
 import * as Icons from '@material-ui/icons'
 import { describe, it, expect, afterEach } from 'vitest'
 
-import WithPopover from './WithPopover'
+import WithPopover, { CloseOnClick } from './WithPopover'
 
 describe('components/Buttons/WithPopover', () => {
   afterEach(cleanup)
@@ -87,24 +87,36 @@ describe('components/Buttons/WithPopover', () => {
     expect(screen.queryByTestId('popup-content')).toBeNull()
   })
 
-  it('should close popup when paper is clicked', () => {
-    const { container } = render(
+  it('should keep popup open when content inside it is clicked', () => {
+    render(
       <WithPopover label="Test Button">
         <div data-testid="popup-content">Popup Content</div>
       </WithPopover>,
     )
 
     const button = screen.getByRole('button', { name: /test button/i })
     fireEvent.click(button)
+    expect(screen.getByTestId('popup-content')).toBeTruthy()
 
+    fireEvent.click(screen.getByTestId('popup-content'))
     expect(screen.getByTestId('popup-content')).toBeTruthy()
+  })
 
-    const paper = container.querySelector('.MuiPaper-root')
-    if (paper) {
-      fireEvent.click(paper)
-    }
+  it('should close popup when CloseOnClick child is clicked', () => {
+    render(
+      <WithPopover label="Test Button">
+        <CloseOnClick>
+          <button data-testid="action-button">Do something</button>
+        </CloseOnClick>
+      </WithPopover>,
+    )
 
-    expect(screen.queryByTestId('popup-content')).toBeNull()
+    const trigger = screen.getByRole('button', { name: /test button/i })
+    fireEvent.click(trigger)
+    expect(screen.getByTestId('action-button')).toBeTruthy()
+
+    fireEvent.click(screen.getByTestId('action-button'))
+    expect(screen.queryByTestId('action-button')).toBeNull()
   })
 
   it('should toggle popup state on button click', () => {

catalog/app/components/Buttons/WithPopover.tsx

@@ -2,6 +2,8 @@ import * as React from 'react'
 import * as M from '@material-ui/core'
 import * as Icons from '@material-ui/icons'
 
+import log from 'utils/Logging'
+
 import Iconized from './Iconized'
 import type { StrIcon, SvgIcon } from './Iconized'
 
@@ -49,6 +51,21 @@ interface WithPopoverPropsOwn {
 export type WithPopoverProps = WithPopoverPropsOwn &
   Omit<Parameters<typeof Iconized>[0], 'icon'>
 
+const CloseContext = React.createContext<() => void>(() =>
+  log.warn('usePopoverClose() called outside of a WithPopover'),
+)
+
+export const useClose = () => React.useContext(CloseContext)
+
+interface CloseOnClickProps {
+  children: React.ReactNode
+}
+
+export function CloseOnClick({ children }: CloseOnClickProps) {
+  const close = useClose()
+  return <div onClick={close}>{children}</div>
+}
+
 export default function WithPopover({
   children,
   icon,
@@ -86,9 +103,11 @@ export default function WithPopover({
 
       <M.Backdrop open={opened} className={classes.backdrop} onClick={handleClose} />
       {opened && (
-        <M.Paper className={classes.popup} elevation={4} onClick={handleClose}>
-          {children}
-        </M.Paper>
+        <CloseContext.Provider value={handleClose}>
+          <M.Paper className={classes.popup} elevation={4}>
+            {children}
+          </M.Paper>
+        </CloseContext.Provider>
       )}
     </div>
   )

catalog/app/components/Buttons/index.ts

@@ -1,6 +1,10 @@
 export { default as Iconized } from './Iconized'
 export { default as Skeleton } from './Skeleton'
-export { default as WithPopover } from './WithPopover'
+export {
+  default as WithPopover,
+  useClose as usePopoverClose,
+  CloseOnClick,
+} from './WithPopover'
 
 export type { SvgIcon, StrIcon } from './Iconized'
 export type { WithPopoverProps } from './WithPopover'

catalog/app/components/Dialog/PopoverOptions.tsx

@@ -38,17 +38,10 @@ interface TabProps {
 
 function Tab({ active, onClick, className, children }: TabProps) {
   const classes = useTabStyles()
-  const handleClick = React.useCallback(
-    (event: React.MouseEvent<HTMLButtonElement>) => {
-      event.stopPropagation()
-      onClick()
-    },
-    [onClick],
-  )
   return (
     <M.Button
       className={cx(classes.root, active && classes.active, className)}
-      onClick={handleClick}
+      onClick={onClick}
     >
       {children}
     </M.Button>

catalog/app/containers/Bucket/Dir/Toolbar/Add/Options.tsx

@@ -2,6 +2,7 @@ import * as React from 'react'
 import * as M from '@material-ui/core'
 import * as Icons from '@material-ui/icons'
 
+import { CloseOnClick } from 'components/Buttons'
 import * as Context from './Context'
 
 const LIST_ITEM_TYPOGRAPHY_PROPS = { noWrap: true } as const
@@ -27,17 +28,19 @@ function MenuItem({ icon, primary, onClick }: MenuItemProps) {
 export default function AddOptions() {
   const { createFile, openUploadDialog } = Context.use()
   return (
-    <M.List dense>
-      <MenuItem
-        icon={<Icons.CreateOutlined />}
-        primary="Create text file"
-        onClick={createFile}
-      />
-      <MenuItem
-        icon={<Icons.PublishOutlined />}
-        primary="Upload files"
-        onClick={openUploadDialog}
-      />
-    </M.List>
+    <CloseOnClick>
+      <M.List dense>
+        <MenuItem
+          icon={<Icons.CreateOutlined />}
+          primary="Create text file"
+          onClick={createFile}
+        />
+        <MenuItem
+          icon={<Icons.PublishOutlined />}
+          primary="Upload files"
+          onClick={openUploadDialog}
+        />
+      </M.List>
+    </CloseOnClick>
   )
 }

catalog/app/containers/Bucket/Dir/Toolbar/CreatePackage/Options.tsx

@@ -2,6 +2,8 @@ import * as React from 'react'
 import * as M from '@material-ui/core'
 import * as Lab from '@material-ui/lab'
 
+import { CloseOnClick } from 'components/Buttons'
+
 import { EmptySlot, ErrorSlot } from 'containers/Bucket/Successors'
 import * as Request from 'utils/useRequest'
 import * as workflows from 'utils/workflows'
@@ -59,24 +61,19 @@ export default function CreatePackageOptions({
   }
 
   return (
-    <M.List dense className={classes.root}>
-      {successors.map((successor) => (
-        <M.ListItem
-          key={successor.slug}
-          onClick={(event) => {
-            event.stopPropagation()
-            onChange(successor)
-          }}
-          button
-        >
-          <M.ListItemText
-            primary={successor.name}
-            primaryTypographyProps={LIST_ITEM_TYPOGRAPHY_PROPS}
-            secondary={successor.url}
-            secondaryTypographyProps={LIST_ITEM_TYPOGRAPHY_PROPS}
-          />
-        </M.ListItem>
-      ))}
-    </M.List>
+    <CloseOnClick>
+      <M.List dense className={classes.root}>
+        {successors.map((successor) => (
+          <M.ListItem key={successor.slug} onClick={() => onChange(successor)} button>
+            <M.ListItemText
+              primary={successor.name}
+              primaryTypographyProps={LIST_ITEM_TYPOGRAPHY_PROPS}
+              secondary={successor.url}
+              secondaryTypographyProps={LIST_ITEM_TYPOGRAPHY_PROPS}
+            />
+          </M.ListItem>
+        ))}
+      </M.List>
+    </CloseOnClick>
   )
 }

catalog/app/containers/Bucket/Dir/Toolbar/Get/Options.tsx

@@ -41,19 +41,11 @@ interface DownloadDirProps {
 
 function DownloadDir({ dirHandle }: DownloadDirProps) {
   // TODO: pass selection to Buttons.DownloadDir
-  const [downloading, setDownloading] = React.useState(false)
-  React.useEffect(() => {
-    if (!downloading) return
-    setTimeout(() => setDownloading(false), 1000)
-  }, [downloading])
+  const feedback = Buttons.useDownloadFeedback()
   return (
     <Buttons.DownloadDir
       suffix={`dir/${dirHandle.bucket}/${dirHandle.path}`}
-      onClick={(event) => {
-        event.stopPropagation()
-        setDownloading(true)
-      }}
-      {...(downloading ? { startIcon: <M.CircularProgress size={20} /> } : null)}
+      {...feedback}
     >
       Download ZIP (directory)
     </Buttons.DownloadDir>

catalog/app/containers/Bucket/Dir/Toolbar/Organize/Options.tsx

@@ -2,6 +2,7 @@ import * as React from 'react'
 import * as M from '@material-ui/core'
 import * as Icons from '@material-ui/icons'
 
+import { CloseOnClick } from 'components/Buttons'
 import * as Format from 'utils/format'
 import assertNever from 'utils/assertNever'
 
@@ -77,7 +78,7 @@ export default function OrganizeOptions({ features }: OrganizeOptionsProps) {
   }, [bookmarkStatus])
 
   return (
-    <>
+    <CloseOnClick>
       <M.ListSubheader inset component="div" disableSticky>
         <Format.Plural
           value={selectionCount}
@@ -121,6 +122,6 @@ export default function OrganizeOptions({ features }: OrganizeOptionsProps) {
           </M.List>
         </>
       )}
-    </>
+    </CloseOnClick>
   )
 }

catalog/app/containers/Bucket/Download/Buttons.tsx

@@ -2,11 +2,35 @@ import * as React from 'react'
 import * as M from '@material-ui/core'
 import * as Icons from '@material-ui/icons'
 
+import { usePopoverClose } from 'components/Buttons'
 import type * as Model from 'model'
 import * as AWS from 'utils/AWS'
 
 import { ZipDownloadForm } from '../FileView'
 
+export function useDownloadFeedback(): {
+  onClick: () => void
+  startIcon?: React.ReactNode
+} {
+  const closePopover = usePopoverClose()
+  const [downloading, setDownloading] = React.useState(false)
+  React.useEffect(() => {
+    if (!downloading) return
+    const timer = setTimeout(() => {
+      setDownloading(false)
+      closePopover()
+    }, 1000)
+    return () => clearTimeout(timer)
+  }, [downloading, closePopover])
+  return React.useMemo(
+    () => ({
+      onClick: () => setDownloading(true),
+      ...(downloading ? { startIcon: <M.CircularProgress size={20} /> } : null),
+    }),
+    [downloading],
+  )
+}
+
 const useDownloadButtonStyles = M.makeStyles({
   root: {
     justifyContent: 'flex-start',
@@ -19,7 +43,10 @@ interface DownloadFileProps {
   fileHandle: Model.S3.S3ObjectLocation
 }
 
-export function DownloadFile({ fileHandle }: DownloadFileProps) {
+export function DownloadFile({
+  fileHandle,
+  ...props
+}: DownloadFileProps & M.ButtonProps<'a'>) {
   const url = AWS.Signer.useDownloadUrl(fileHandle)
   const classes = useDownloadButtonStyles()
   return (
@@ -28,6 +55,7 @@ export function DownloadFile({ fileHandle }: DownloadFileProps) {
       download
       href={url}
       startIcon={<Icons.ArrowDownwardOutlined />}
+      {...props}
     >
       Download file
     </M.Button>