fix(SelectPanel2): only bind keydown event when necessary (#4601)

* fix(SelectPanel2): only bind keydown event when necessary

* chore(SelectPanel2): Add changeset

Author: bwittenberg

.changeset/good-bugs-grab.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+SelectPanel2: Minor optimization for escape key event listener binding

packages/react/src/drafts/SelectPanel2/SelectPanel.test.tsx

@@ -179,6 +179,38 @@ describe('SelectPanel', () => {
     expect(mockOnSubmit).toHaveBeenCalledTimes(0)
   })
 
+  it('should not call addEventListener on each render for Escape key handling when onCancel has not changed', async () => {
+    const onCancel = jest.fn()
+    const container = render(
+      <SelectPanel title="title" onCancel={onCancel}>
+        child
+      </SelectPanel>,
+    )
+    const addEventListenerSpy = jest.spyOn(globalThis.EventTarget.prototype, 'addEventListener')
+    const removeEventListenerSpy = jest.spyOn(globalThis.EventTarget.prototype, 'removeEventListener')
+
+    container.rerender(
+      <SelectPanel title="title" onCancel={onCancel}>
+        child
+      </SelectPanel>,
+    )
+    expect(addEventListenerSpy).not.toHaveBeenCalled()
+    expect(removeEventListenerSpy).not.toHaveBeenCalled()
+  })
+
+  it('Escape key closes the dialog and calls onCancel', async () => {
+    const mockOnSubmit = jest.fn()
+    const mockOnCancel = jest.fn()
+    const {container, user} = await getFixtureWithOpenContainer({mockOnSubmit, mockOnCancel})
+    selectUnselectedOption(container, user)
+
+    await user.keyboard('{Escape}')
+
+    expect(container.queryByRole('dialog')).toBeNull()
+    expect(mockOnCancel).toHaveBeenCalledTimes(1)
+    expect(mockOnSubmit).toHaveBeenCalledTimes(0)
+  })
+
   it('SelectPanel within FormControl should be labelled by FormControl.Label', async () => {
     const component = render(<SelectPanelWithinForm />)
     const buttonByRole = component.getByRole('button')

packages/react/src/drafts/SelectPanel2/SelectPanel.tsx

@@ -136,10 +136,10 @@ const Panel: React.FC<SelectPanelProps> = ({
     if (propsOpen === undefined) setInternalOpen(false)
   }, [internalOpen, propsOpen])
 
-  const onInternalCancel = () => {
+  const onInternalCancel = React.useCallback(() => {
     onInternalClose()
     if (typeof propsOnCancel === 'function') propsOnCancel()
-  }
+  }, [onInternalClose, propsOnCancel])
 
   const onInternalSubmit = (event?: React.FormEvent<HTMLFormElement>) => {
     event?.preventDefault() // there is no event with selectionVariant=instant
@@ -199,7 +199,7 @@ const Panel: React.FC<SelectPanelProps> = ({
     }
     dialogEl?.addEventListener('keydown', handler)
     return () => dialogEl?.removeEventListener('keydown', handler)
-  })
+  }, [onInternalCancel])
 
   // Autofocus hack: React doesn't support autoFocus for dialog: https://github.com/facebook/react/issues/23301
   // tl;dr: react takes over autofocus instead of letting the browser handle it,