react icon indicating copy to clipboard operation
react copied to clipboard

fix(SelectPanel2): only bind keydown event when necessary

Open bwittenberg opened this issue 1 year ago • 2 comments

Closes #4581

Changelog

New

  • Adds additional jest test coverage for the SelectPanel

Changed

  • Adds optimization to SelectPanel so that event listeners are not added and removed on each render, but only when the effect dependencies change.

Removed

Rollout strategy

  • [x] Patch release
  • [ ] Minor release
  • [ ] Major release; if selected, include a written rollout or migration plan
  • [ ] None; if selected, include a brief description as to why

Testing & Reviewing

  • One of the SelectPanel jest test should fail if changes to SelectPanel are reverted.
  • RT1: Regression Test 1: Panel should close when opened and escape is pressed
    • Visit the SelectPanel in storybook (http:///?path=/story/drafts-components-selectpanel--default)
    • Click the button, assert panel opens
    • Press the Escape key on the keyboard, assert panel closes

Merge checklist

  • [x] Added/updated tests ~- [ ] Added/updated documentation~ ~- [ ] Added/updated previews (Storybook)~
  • [ ] Changes are SSR compatible
    • I'm not sure about this one. Because hooks with dependencies are in use already, this code change shouldn't introduce any issues. I didn't test though, which is why I'm not checking this box.
  • [x] Tested in Chrome (ran RT1 above on Chrome 124.0.6367.207)
  • [x] Tested in Firefox (ran RT1 above on Firefox 126.0)
  • [x] Tested in Safari (ran RT1 on Safari 17.4.1)
  • [x] Tested in Edge (ran RT1 on Edge 124.0.2478.105)
  • [ ] (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

bwittenberg avatar May 16 '24 17:05 bwittenberg

🦋 Changeset detected

Latest commit: f332218ed8bc772b4e15d53959a2c66df7fb7e46

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar May 16 '24 17:05 changeset-bot[bot]

Hello @broccolinisoup! Appreciate the 👀. If you'd like to see any changes, I'm happy to make revisions.

bwittenberg avatar May 17 '24 14:05 bwittenberg