From 53ff7baa908d167ec931fe4d8acb8cd5199cba8d Mon Sep 17 00:00:00 2001
From: Siddharth Kshetrapal <siddharthkp@github.com>
Date: Tue, 5 Nov 2024 12:34:00 +0100
Subject: [PATCH 1/5] use Overlay instead of AnchoredOverlay

---
 .../react/src/SelectPanel/SelectPanel.tsx     | 215 +++++++++++-------
 1 file changed, 134 insertions(+), 81 deletions(-)

diff --git a/packages/react/src/SelectPanel/SelectPanel.tsx b/packages/react/src/SelectPanel/SelectPanel.tsx
index 98a7a66d322..49078f77749 100644
--- a/packages/react/src/SelectPanel/SelectPanel.tsx
+++ b/packages/react/src/SelectPanel/SelectPanel.tsx
@@ -1,7 +1,7 @@
 import {SearchIcon, TriangleDownIcon} from '@primer/octicons-react'
 import React, {useCallback, useMemo} from 'react'
 import type {AnchoredOverlayProps} from '../AnchoredOverlay'
-import {AnchoredOverlay} from '../AnchoredOverlay'
+import Overlay from '../Overlay'
 import type {AnchoredOverlayWrapperAnchorProps} from '../AnchoredOverlay/AnchoredOverlay'
 import Box from '../Box'
 import type {FilteredActionListProps} from '../FilteredActionList'
@@ -12,13 +12,14 @@ import type {TextInputProps} from '../TextInput'
 import type {ItemProps, ItemInput} from './types'
 
 import {Button} from '../Button'
-import {useProvidedRefOrCreate} from '../hooks'
-import type {FocusZoneHookSettings} from '../hooks/useFocusZone'
+import {useAnchoredPosition, useProvidedRefOrCreate} from '../hooks'
 import {useId} from '../hooks/useId'
 import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
 import {LiveRegion, LiveRegionOutlet, Message} from '../internal/components/LiveRegion'
 import {useFeatureFlag} from '../FeatureFlags'
 
+import {useFocusTrap} from '../hooks/useFocusTrap'
+
 interface SelectPanelSingleSelection {
   selected: ItemInput | undefined
   onSelectedChange: (selected: ItemInput | undefined) => void
@@ -56,11 +57,6 @@ function isMultiSelectVariant(
   return Array.isArray(selected)
 }
 
-const focusZoneSettings: Partial<FocusZoneHookSettings> = {
-  // Let FilteredActionList handle focus zone
-  disabled: true,
-}
-
 const areItemsEqual = (itemA: ItemInput, itemB: ItemInput) => {
   // prefer checking equivality by item.id
   if (typeof itemA.id !== 'undefined') return itemA.id === itemB.id
@@ -137,6 +133,65 @@ export function SelectPanel({
     }
   }, [placeholder, renderAnchor, selected])
 
+  /* Anchoring logic */
+  const overlayRef = React.useRef<HTMLDivElement>(null)
+  const inputRef = React.useRef<HTMLInputElement>(null)
+
+  const {position} = useAnchoredPosition(
+    {
+      anchorElementRef: anchorRef,
+      floatingElementRef: overlayRef,
+      side: 'outside-bottom',
+      align: 'start',
+    },
+    [open, anchorRef.current, overlayRef.current],
+  )
+
+  const onAnchorClick = useCallback(
+    (event: React.MouseEvent<HTMLElement>) => {
+      if (event.defaultPrevented || event.button !== 0) {
+        return
+      }
+
+      if (!open) {
+        onOpen('anchor-click')
+      } else {
+        onClose('anchor-click')
+      }
+    },
+    [open, onOpen, onClose],
+  )
+
+  const onAnchorKeyDown = useCallback(
+    (event: React.KeyboardEvent<HTMLElement>) => {
+      if (!event.defaultPrevented) {
+        if (!open && ['ArrowDown', 'ArrowUp', ' ', 'Enter'].includes(event.key)) {
+          onOpen('anchor-key-press', event)
+          event.preventDefault()
+        }
+      }
+    },
+    [open, onOpen],
+  )
+
+  const anchorProps = {
+    ref: anchorRef,
+    onClick: onAnchorClick,
+    'aria-haspopup': true,
+    'aria-expanded': open,
+    onKeyDown: onAnchorKeyDown,
+  }
+  // TODO: anchor should be called button because it's not an anchor anymore
+  const anchor = renderMenuAnchor ? renderMenuAnchor(anchorProps) : null
+
+  /** Focus trap */
+  useFocusTrap({
+    containerRef: overlayRef,
+    disabled: !open || !position,
+    returnFocusRef: anchorRef,
+    initialFocusRef: inputRef,
+  })
+
   const itemsToRender = useMemo(() => {
     return items.map(item => {
       const isItemSelected = isMultiSelectVariant(selected) ? doesItemsIncludeItem(selected, item) : selected === item
@@ -172,11 +227,6 @@ export function SelectPanel({
     })
   }, [onClose, onSelectedChange, items, selected])
 
-  const inputRef = React.useRef<HTMLInputElement>(null)
-  const focusTrapSettings = {
-    initialFocusRef: inputRef,
-  }
-
   const extendedTextInputProps: Partial<TextInputProps> = useMemo(() => {
     return {
       sx: {m: 2},
@@ -191,76 +241,79 @@ export function SelectPanel({
 
   return (
     <LiveRegion>
-      <AnchoredOverlay
-        renderAnchor={renderMenuAnchor}
-        anchorRef={anchorRef}
-        open={open}
-        onOpen={onOpen}
-        onClose={onClose}
-        overlayProps={{
-          role: 'dialog',
-          'aria-labelledby': titleId,
-          'aria-describedby': subtitle ? subtitleId : undefined,
-          ...overlayProps,
-        }}
-        focusTrapSettings={focusTrapSettings}
-        focusZoneSettings={focusZoneSettings}
-      >
-        <LiveRegionOutlet />
-        {usingModernActionList ? null : (
-          <Message
-            value={
-              filterValue === ''
-                ? 'Showing all items'
-                : items.length <= 0
-                  ? 'No matching items'
-                  : `${items.length} matching ${items.length === 1 ? 'item' : 'items'}`
-            }
-          />
-        )}
-        <Box sx={{display: 'flex', flexDirection: 'column', height: 'inherit', maxHeight: 'inherit'}}>
-          <Box sx={{pt: 2, px: 3}}>
-            <Heading as="h1" id={titleId} sx={{fontSize: 1}}>
-              {title}
-            </Heading>
-            {subtitle ? (
-              <Box id={subtitleId} sx={{fontSize: 0, color: 'fg.muted'}}>
-                {subtitle}
+      {anchor}
+      {open && (
+        <Overlay
+          role="dialog"
+          aria-labelledby={titleId}
+          aria-describedby={subtitle ? subtitleId : undefined}
+          ref={overlayRef}
+          returnFocusRef={anchorRef}
+          onEscape={() => onClose('escape')}
+          onClickOutside={() => onClose('click-outside')}
+          ignoreClickRefs={
+            /* this is required so that clicking the button while the panel is open does not re-open the panel */
+            [anchorRef]
+          }
+          {...position}
+          {...overlayProps}
+        >
+          <LiveRegionOutlet />
+          {usingModernActionList ? null : (
+            <Message
+              value={
+                filterValue === ''
+                  ? 'Showing all items'
+                  : items.length <= 0
+                    ? 'No matching items'
+                    : `${items.length} matching ${items.length === 1 ? 'item' : 'items'}`
+              }
+            />
+          )}
+          <Box sx={{display: 'flex', flexDirection: 'column', height: 'inherit', maxHeight: 'inherit'}}>
+            <Box sx={{pt: 2, px: 3}}>
+              <Heading as="h1" id={titleId} sx={{fontSize: 1}}>
+                {title}
+              </Heading>
+              {subtitle ? (
+                <Box id={subtitleId} sx={{fontSize: 0, color: 'fg.muted'}}>
+                  {subtitle}
+                </Box>
+              ) : null}
+            </Box>
+            <FilteredActionList
+              filterValue={filterValue}
+              onFilterChange={onFilterChange}
+              placeholderText={placeholderText}
+              {...listProps}
+              role="listbox"
+              // browsers give aria-labelledby precedence over aria-label so we need to make sure
+              // we don't accidentally override props.aria-label
+              aria-labelledby={listProps['aria-label'] ? undefined : titleId}
+              aria-multiselectable={isMultiSelectVariant(selected) ? 'true' : 'false'}
+              selectionVariant={isMultiSelectVariant(selected) ? 'multiple' : 'single'}
+              items={itemsToRender}
+              textInputProps={extendedTextInputProps}
+              inputRef={inputRef}
+              // inheriting height and maxHeight ensures that the FilteredActionList is never taller
+              // than the Overlay (which would break scrolling the items)
+              sx={{...sx, height: 'inherit', maxHeight: 'inherit'}}
+            />
+            {footer && (
+              <Box
+                sx={{
+                  display: 'flex',
+                  borderTop: '1px solid',
+                  borderColor: 'border.default',
+                  padding: 2,
+                }}
+              >
+                {footer}
               </Box>
-            ) : null}
+            )}
           </Box>
-          <FilteredActionList
-            filterValue={filterValue}
-            onFilterChange={onFilterChange}
-            placeholderText={placeholderText}
-            {...listProps}
-            role="listbox"
-            // browsers give aria-labelledby precedence over aria-label so we need to make sure
-            // we don't accidentally override props.aria-label
-            aria-labelledby={listProps['aria-label'] ? undefined : titleId}
-            aria-multiselectable={isMultiSelectVariant(selected) ? 'true' : 'false'}
-            selectionVariant={isMultiSelectVariant(selected) ? 'multiple' : 'single'}
-            items={itemsToRender}
-            textInputProps={extendedTextInputProps}
-            inputRef={inputRef}
-            // inheriting height and maxHeight ensures that the FilteredActionList is never taller
-            // than the Overlay (which would break scrolling the items)
-            sx={{...sx, height: 'inherit', maxHeight: 'inherit'}}
-          />
-          {footer && (
-            <Box
-              sx={{
-                display: 'flex',
-                borderTop: '1px solid',
-                borderColor: 'border.default',
-                padding: 2,
-              }}
-            >
-              {footer}
-            </Box>
-          )}
-        </Box>
-      </AnchoredOverlay>
+        </Overlay>
+      )}
     </LiveRegion>
   )
 }

From 859019e33353421de354e0c6831e9641d4237df1 Mon Sep 17 00:00:00 2001
From: Siddharth Kshetrapal <siddharthkp@github.com>
Date: Thu, 7 Nov 2024 12:21:34 +0100
Subject: [PATCH 2/5] Create fluffy-days-brake.md

---
 .changeset/fluffy-days-brake.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 .changeset/fluffy-days-brake.md

diff --git a/.changeset/fluffy-days-brake.md b/.changeset/fluffy-days-brake.md
new file mode 100644
index 00000000000..a965179d8da
--- /dev/null
+++ b/.changeset/fluffy-days-brake.md
@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+SelectPanel: (Implementation detail, should have no changes for users) Replace AnchoredOverlay with Overlay and useAnchoredPosition

From e427df020e4b2fb6c3e31d0cc34d2b7bcd9e5d5a Mon Sep 17 00:00:00 2001
From: Siddharth Kshetrapal <siddharthkp@github.com>
Date: Thu, 7 Nov 2024 12:22:02 +0100
Subject: [PATCH 3/5] the tiniest chnage

---
 packages/react/src/SelectPanel/SelectPanel.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/react/src/SelectPanel/SelectPanel.tsx b/packages/react/src/SelectPanel/SelectPanel.tsx
index 49078f77749..5f42011f6e4 100644
--- a/packages/react/src/SelectPanel/SelectPanel.tsx
+++ b/packages/react/src/SelectPanel/SelectPanel.tsx
@@ -176,9 +176,9 @@ export function SelectPanel({
 
   const anchorProps = {
     ref: anchorRef,
-    onClick: onAnchorClick,
     'aria-haspopup': true,
     'aria-expanded': open,
+    onClick: onAnchorClick,
     onKeyDown: onAnchorKeyDown,
   }
   // TODO: anchor should be called button because it's not an anchor anymore

From fca7e792d4388c8ffb1a7fc88d404f1e7b97b961 Mon Sep 17 00:00:00 2001
From: Siddharth Kshetrapal <siddharthkp@github.com>
Date: Thu, 7 Nov 2024 12:24:57 +0100
Subject: [PATCH 4/5] move focus trap for easier review

---
 packages/react/src/SelectPanel/SelectPanel.tsx | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/packages/react/src/SelectPanel/SelectPanel.tsx b/packages/react/src/SelectPanel/SelectPanel.tsx
index 5f42011f6e4..ecef97b4c6e 100644
--- a/packages/react/src/SelectPanel/SelectPanel.tsx
+++ b/packages/react/src/SelectPanel/SelectPanel.tsx
@@ -184,14 +184,6 @@ export function SelectPanel({
   // TODO: anchor should be called button because it's not an anchor anymore
   const anchor = renderMenuAnchor ? renderMenuAnchor(anchorProps) : null
 
-  /** Focus trap */
-  useFocusTrap({
-    containerRef: overlayRef,
-    disabled: !open || !position,
-    returnFocusRef: anchorRef,
-    initialFocusRef: inputRef,
-  })
-
   const itemsToRender = useMemo(() => {
     return items.map(item => {
       const isItemSelected = isMultiSelectVariant(selected) ? doesItemsIncludeItem(selected, item) : selected === item
@@ -227,6 +219,14 @@ export function SelectPanel({
     })
   }, [onClose, onSelectedChange, items, selected])
 
+  /** Focus trap */
+  useFocusTrap({
+    containerRef: overlayRef,
+    disabled: !open || !position,
+    initialFocusRef: inputRef,
+    returnFocusRef: anchorRef,
+  })
+
   const extendedTextInputProps: Partial<TextInputProps> = useMemo(() => {
     return {
       sx: {m: 2},

From f0be48080b0f2fbeee2abb1334f08ef87851b85f Mon Sep 17 00:00:00 2001
From: Siddharth Kshetrapal <siddharthkp@github.com>
Date: Thu, 7 Nov 2024 12:31:08 +0100
Subject: [PATCH 5/5] tiny formatting to make it easier to review

---
 .../react/src/SelectPanel/SelectPanel.tsx     | 142 +++++++++---------
 1 file changed, 71 insertions(+), 71 deletions(-)

diff --git a/packages/react/src/SelectPanel/SelectPanel.tsx b/packages/react/src/SelectPanel/SelectPanel.tsx
index ecef97b4c6e..a94b461e169 100644
--- a/packages/react/src/SelectPanel/SelectPanel.tsx
+++ b/packages/react/src/SelectPanel/SelectPanel.tsx
@@ -17,7 +17,6 @@ import {useId} from '../hooks/useId'
 import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
 import {LiveRegion, LiveRegionOutlet, Message} from '../internal/components/LiveRegion'
 import {useFeatureFlag} from '../FeatureFlags'
-
 import {useFocusTrap} from '../hooks/useFocusTrap'
 
 interface SelectPanelSingleSelection {
@@ -239,81 +238,82 @@ export function SelectPanel({
 
   const usingModernActionList = useFeatureFlag('primer_react_select_panel_with_modern_action_list')
 
+  if (!open) return <>{anchor}</>
+
   return (
     <LiveRegion>
       {anchor}
-      {open && (
-        <Overlay
-          role="dialog"
-          aria-labelledby={titleId}
-          aria-describedby={subtitle ? subtitleId : undefined}
-          ref={overlayRef}
-          returnFocusRef={anchorRef}
-          onEscape={() => onClose('escape')}
-          onClickOutside={() => onClose('click-outside')}
-          ignoreClickRefs={
-            /* this is required so that clicking the button while the panel is open does not re-open the panel */
-            [anchorRef]
-          }
-          {...position}
-          {...overlayProps}
-        >
-          <LiveRegionOutlet />
-          {usingModernActionList ? null : (
-            <Message
-              value={
-                filterValue === ''
-                  ? 'Showing all items'
-                  : items.length <= 0
-                    ? 'No matching items'
-                    : `${items.length} matching ${items.length === 1 ? 'item' : 'items'}`
-              }
-            />
-          )}
-          <Box sx={{display: 'flex', flexDirection: 'column', height: 'inherit', maxHeight: 'inherit'}}>
-            <Box sx={{pt: 2, px: 3}}>
-              <Heading as="h1" id={titleId} sx={{fontSize: 1}}>
-                {title}
-              </Heading>
-              {subtitle ? (
-                <Box id={subtitleId} sx={{fontSize: 0, color: 'fg.muted'}}>
-                  {subtitle}
-                </Box>
-              ) : null}
-            </Box>
-            <FilteredActionList
-              filterValue={filterValue}
-              onFilterChange={onFilterChange}
-              placeholderText={placeholderText}
-              {...listProps}
-              role="listbox"
-              // browsers give aria-labelledby precedence over aria-label so we need to make sure
-              // we don't accidentally override props.aria-label
-              aria-labelledby={listProps['aria-label'] ? undefined : titleId}
-              aria-multiselectable={isMultiSelectVariant(selected) ? 'true' : 'false'}
-              selectionVariant={isMultiSelectVariant(selected) ? 'multiple' : 'single'}
-              items={itemsToRender}
-              textInputProps={extendedTextInputProps}
-              inputRef={inputRef}
-              // inheriting height and maxHeight ensures that the FilteredActionList is never taller
-              // than the Overlay (which would break scrolling the items)
-              sx={{...sx, height: 'inherit', maxHeight: 'inherit'}}
-            />
-            {footer && (
-              <Box
-                sx={{
-                  display: 'flex',
-                  borderTop: '1px solid',
-                  borderColor: 'border.default',
-                  padding: 2,
-                }}
-              >
-                {footer}
+
+      <Overlay
+        role="dialog"
+        aria-labelledby={titleId}
+        aria-describedby={subtitle ? subtitleId : undefined}
+        ref={overlayRef}
+        returnFocusRef={anchorRef}
+        onEscape={() => onClose('escape')}
+        onClickOutside={() => onClose('click-outside')}
+        ignoreClickRefs={
+          /* this is required so that clicking the button while the panel is open does not re-open the panel */
+          [anchorRef]
+        }
+        {...position}
+        {...overlayProps}
+      >
+        <LiveRegionOutlet />
+        {usingModernActionList ? null : (
+          <Message
+            value={
+              filterValue === ''
+                ? 'Showing all items'
+                : items.length <= 0
+                  ? 'No matching items'
+                  : `${items.length} matching ${items.length === 1 ? 'item' : 'items'}`
+            }
+          />
+        )}
+        <Box sx={{display: 'flex', flexDirection: 'column', height: 'inherit', maxHeight: 'inherit'}}>
+          <Box sx={{pt: 2, px: 3}}>
+            <Heading as="h1" id={titleId} sx={{fontSize: 1}}>
+              {title}
+            </Heading>
+            {subtitle ? (
+              <Box id={subtitleId} sx={{fontSize: 0, color: 'fg.muted'}}>
+                {subtitle}
               </Box>
-            )}
+            ) : null}
           </Box>
-        </Overlay>
-      )}
+          <FilteredActionList
+            filterValue={filterValue}
+            onFilterChange={onFilterChange}
+            placeholderText={placeholderText}
+            {...listProps}
+            role="listbox"
+            // browsers give aria-labelledby precedence over aria-label so we need to make sure
+            // we don't accidentally override props.aria-label
+            aria-labelledby={listProps['aria-label'] ? undefined : titleId}
+            aria-multiselectable={isMultiSelectVariant(selected) ? 'true' : 'false'}
+            selectionVariant={isMultiSelectVariant(selected) ? 'multiple' : 'single'}
+            items={itemsToRender}
+            textInputProps={extendedTextInputProps}
+            inputRef={inputRef}
+            // inheriting height and maxHeight ensures that the FilteredActionList is never taller
+            // than the Overlay (which would break scrolling the items)
+            sx={{...sx, height: 'inherit', maxHeight: 'inherit'}}
+          />
+          {footer && (
+            <Box
+              sx={{
+                display: 'flex',
+                borderTop: '1px solid',
+                borderColor: 'border.default',
+                padding: 2,
+              }}
+            >
+              {footer}
+            </Box>
+          )}
+        </Box>
+      </Overlay>
     </LiveRegion>
   )
 }