SelectPanel: Update SelectPanel to use modern ActionList #4794

broccolinisoup · 2024-07-30T04:51:42Z

Note

New feature flag introduced: primer_react_select_panel_with_modern_action_list

What's changed

Closes https://github.com/github/primer/issues/3671
Implements parts of https://github.com/github/primer/issues/3183

This is a pretty scary PR to roll out as it is replacing ActionList used internally in SelectPanel, that is why we are putting everything behind a feature flag (see FilteredActionList "entry file").

There should be no change without the feature flag enabled
Jest tests added for both states of the feature flag
VRT snapshots added for both states of the feature flag
Some accessibility remediations that were made to ActionList and not deprecated/ActionList do cause certain tests to fail in dotcom CI. I have a PR for dotcom updating tests that works with both states of the feature flag (can be merged independently)

Rollout strategy

Patch release (feature flagged)
Minor release
Major release; if selected, include a written rollout or migration plan
None; if selected, include a brief description as to why

Merge checklist

Added/updated tests
Added/updated documentation
Added/updated previews (Storybook)
Changes are SSR compatible
Tested in Chrome
Tested in Firefox
Tested in Safari
Tested in Edge
(GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

Integration tests:

✅ Run integration tests with flag off
✅ Run integration tests with flag on
- Debug why empty item from memex is not rendering
Update tests in dotcom https://github.com/github/github/pull/339699 (can be merged independently)

changeset-bot · 2024-07-30T04:51:46Z

🦋 Changeset detected

Latest commit: 0b57206

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

packages/react/src/FilteredActionList/FilteredActionList.tsx

github-actions · 2024-07-30T05:17:44Z

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	96.86 KB (+0.46% 🔺)
packages/react/dist/browser.umd.js	97.18 KB (+0.55% 🔺)

camertron

Awesome, nice work! As far as groups go, it looks like both the beta and deprecated versions of ActionList support them. Maybe we could create a sub-component called MappedActionListGroup? Then we could do something like:

function MappedActionListGroup({ items, header }: GroupProps) {
  return (
    <ActionList.Group>
      <ActionList.GroupHeading variant={header.variant}>
        {header.title}
      </ActionList.GroupHeading>
      {items.map((item) => <MappedActionList item={item}>)}
    </ActionList.Group>
  )
}

camertron · 2024-08-01T16:55:33Z

packages/react/src/FilteredActionList/FilteredActionList.tsx

+      text,
+      variant,
+      disabled,
+      trailingVisual: TrailingVisual,


I'm not sure if this is a common pattern, but I personally find it a bit confusing to use TitleCase for these renamed variables. Can we avoid renaming them, or name them something else, eg. originalTrailingVisual?

👋

the TitleCase is so that we can render them as a component in JSX

prop accepted as leadingVisual, rendered in the body as <LeadingVisual/>, it would have been nicer if the prop was already TitleCased to signal that it accepts a component (and not a string, for example), but probably not the correct time to change that here

packages/react/src/FilteredActionList/FilteredActionList.tsx

joshblack

LGTM! Just had a question

joshblack · 2024-09-06T16:46:12Z

packages/react/src/FilteredActionList/FilteredActionListWithModernActionList.tsx

+    if (activeDescendantRef.current && scrollContainerRef.current) {
+      scrollIntoView(activeDescendantRef.current, scrollContainerRef.current, {...menuScrollMargins, behavior: 'auto'})
+    }
+  }, [items])


Is items a memoized value or is a plain value typically?

They are memoized in SelectPanel before passing to FilteredActionList. Tell me more?

joshblack · 2024-09-06T16:48:10Z

Also wanted to leave a question, would it be possible for this to go in the next release batch? (I think it would be rc.5)

Would help out a ton on my end, the release is starting to get big 😅

siddharthkp · 2024-09-09T10:07:59Z

Also wanted to leave a question, would it be possible for this to go in the next release batch? (I think it would be rc.5)

Absolutely! I can wait :)

Update: rc.4 was merged yesterday, can merge this now 🎉

broccolinisoup · 2024-09-12T20:46:21Z

So happy to see this is merged!! Thanks @siddharthkp 🥹 💖

super wip

9cee0a4

github-actions bot added the staff Author is a staff member label Jul 30, 2024

broccolinisoup changed the title ~~super wip~~ Ignore - super wip selectpanel Jul 30, 2024

just use the actionlist component and revert the type updates

c7aa5de

broccolinisoup commented Jul 30, 2024

View reviewed changes

packages/react/src/FilteredActionList/FilteredActionList.tsx Outdated Show resolved Hide resolved

Update packages/react/src/FilteredActionList/FilteredActionList.tsx

40a0c45

broccolinisoup temporarily deployed to github-pages July 30, 2024 05:18 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4794 July 30, 2024 05:18 Inactive

Some more progress

344f6ce

broccolinisoup temporarily deployed to github-pages July 31, 2024 02:20 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4794 July 31, 2024 02:21 Inactive

revert the type changes and cast it :/

fa5f5a6

broccolinisoup temporarily deployed to github-pages July 31, 2024 04:09 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4794 July 31, 2024 04:09 Inactive

clean

3cd9ecc

broccolinisoup temporarily deployed to github-pages July 31, 2024 05:35 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4794 July 31, 2024 05:35 Inactive

broccolinisoup added the skip changeset This change does not need a changelog label Jul 31, 2024

broccolinisoup changed the title ~~Ignore - super wip selectpanel~~ SelectPanel: Update SelectPanel to use modern ActionList Aug 1, 2024

camertron reviewed Aug 1, 2024

View reviewed changes

broccolinisoup added 6 commits August 2, 2024 13:11

wip

0003e59

wip wip

713b5ad

add stories

6fc7f2b

fix linting

a246981

add tests for groups

e209f47

Merge branch 'main' into select-panel-action-list

26cddc1

broccolinisoup temporarily deployed to github-pages August 8, 2024 07:09 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4794 August 8, 2024 07:10 Inactive

siddharthkp added the update snapshots label Sep 3, 2024

test(vrt): update snapshots

f72aa55

github-actions bot removed the update snapshots label Sep 3, 2024

primer bot temporarily deployed to github-pages September 3, 2024 10:58 Inactive

github-actions bot temporarily deployed to storybook-preview-4794 September 3, 2024 10:58 Inactive

This was referenced Sep 3, 2024

(waiting) Select Panel: Add home and end keys for keyboard navigation #4903

Closed

Select Panel: Functional adjustments #4906

Merged

set active styles for both active-descendant types

6e9d706

siddharthkp added the update snapshots label Sep 5, 2024

siddharthkp temporarily deployed to github-pages September 5, 2024 10:24 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-4794 September 5, 2024 10:24 Inactive

test(vrt): update snapshots

0b57206

github-actions bot removed the update snapshots label Sep 5, 2024

primer bot deployed to github-pages September 5, 2024 10:33 View deployment

github-actions bot deployed to storybook-preview-4794 September 5, 2024 10:33 View deployment

siddharthkp mentioned this pull request Sep 5, 2024

Select Panel: Add built-in "No matches" item #4920

Closed

13 tasks

joshblack approved these changes Sep 6, 2024

View reviewed changes

siddharthkp changed the title ~~SelectPanel: Update SelectPanel to use modern ActionList~~ (waiting for rc.5) SelectPanel: Update SelectPanel to use modern ActionList Sep 9, 2024

siddharthkp mentioned this pull request Sep 9, 2024

WIP: Select panel announcements #4927

Closed

camertron mentioned this pull request Sep 9, 2024

Select panel loading states #4929

Merged

12 tasks

siddharthkp changed the title ~~(waiting for rc.5) SelectPanel: Update SelectPanel to use modern ActionList~~ SelectPanel: Update SelectPanel to use modern ActionList Sep 10, 2024

siddharthkp added this pull request to the merge queue Sep 10, 2024

Merged via the queue into main with commit 5f996c6 Sep 10, 2024
32 checks passed

siddharthkp deleted the select-panel-action-list branch September 10, 2024 10:07

primer bot mentioned this pull request Sep 10, 2024

Release tracking (rc.5) #4942

Merged

primer bot mentioned this pull request Oct 18, 2024

Release tracking (37.0.0) #5143

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SelectPanel: Update SelectPanel to use modern ActionList #4794

SelectPanel: Update SelectPanel to use modern ActionList #4794

broccolinisoup commented Jul 30, 2024 •

edited by siddharthkp

Loading

changeset-bot bot commented Jul 30, 2024 •

edited

Loading

github-actions bot commented Jul 30, 2024 •

edited

Loading

camertron left a comment

camertron Aug 1, 2024

siddharthkp Aug 30, 2024

joshblack left a comment

joshblack Sep 6, 2024

siddharthkp Sep 9, 2024

joshblack commented Sep 6, 2024

siddharthkp commented Sep 9, 2024 •

edited

Loading

broccolinisoup commented Sep 12, 2024

SelectPanel: Update SelectPanel to use modern ActionList #4794

SelectPanel: Update SelectPanel to use modern ActionList #4794

Conversation

broccolinisoup commented Jul 30, 2024 • edited by siddharthkp Loading

What's changed

Rollout strategy

Merge checklist

changeset-bot bot commented Jul 30, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Jul 30, 2024 • edited Loading

size-limit report 📦

camertron left a comment

Choose a reason for hiding this comment

camertron Aug 1, 2024

Choose a reason for hiding this comment

siddharthkp Aug 30, 2024

Choose a reason for hiding this comment

joshblack left a comment

Choose a reason for hiding this comment

joshblack Sep 6, 2024

Choose a reason for hiding this comment

siddharthkp Sep 9, 2024

Choose a reason for hiding this comment

joshblack commented Sep 6, 2024

siddharthkp commented Sep 9, 2024 • edited Loading

broccolinisoup commented Sep 12, 2024

broccolinisoup commented Jul 30, 2024 •

edited by siddharthkp

Loading

changeset-bot bot commented Jul 30, 2024 •

edited

Loading

github-actions bot commented Jul 30, 2024 •

edited

Loading

siddharthkp commented Sep 9, 2024 •

edited

Loading