ActionList Composable API #1517

siddharthkp · 2021-10-15T01:31:06Z

Ready for review :)

Closes https://github.com/github/primer/issues/307 | Interface guidelines | Figma
Next: https://github.com/github/primer/issues/447 + https://github.com/github/primer/issues/448

See Docs for API surface area for ActionList

See Storybook for use cases as standalone and as List inside Menu/Select

API opinions worth noting:

Watch video instead

Prefer composition over config: See https://github.com/github/primer/issues/307 for long version
selectionVariant for ActionList is null by default. Item makes no assumptions about what kind of selection to render if it is not set on the List root or Group
showDividers is on ActionList root, not each ActionList.Item. Loosely held opinion, will test with memex. The Dividers are meant to add a separator between Items of the list and should be consistent in a given List and not customisable at the Item level.
- Should it also be on Group level? Can 2 groups inside a List have different divider settings?
- Does the name showDividers cause confusion/conflict with ActionList.Divider?
Added ActionList.LinkItem which uses an internal API with Item to make sure we can give the semantics for free.

Progressive improvements:

Try out rc version in memex https://github.com/github/memex/pull/5862
Play well with AnchoredOverlay for "build your own ActionMenu" mode in memex
replace custom item themes when Release Tracking primitives#263 is released
help wanted: typescript unhappy with polymorphic + forwardRef
ActionList.LinkItem
- Polymorphic Link component
- Padding at the container not root
Test suite for create-slots (it's where all the complex logic sits)

Questions:

Design

padding: 8 or 6? x:8, y:6
different size for icon and avatar, range?
Async results UX → https://github.com/github/primer/issues/457

Bundling

Bundle ActionList2 in @primer/components/unreleased

While I'm confident about the API in this PR, I'd like to "release" these changes along with their composable counterparts ActionMenu and SelectPanel, so that the memex team can catch up with breaking API for Menus together. This also gives us a change to tweak and polish the underlying ActionList to be accessible. The change is not code aesthetics, it's a more mature group of components ✨

Accessibility questions moved to ActionList beta audit

Merge checklist

changeset-bot · 2021-10-15T01:31:08Z

🦋 Changeset detected

Latest commit: 3cfc4f7

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

This PR includes changesets to release 1 package

Name	Type
@primer/components	Minor

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

github-actions · 2021-10-15T01:47:36Z

size-limit report 📦

Path	Size
dist/browser.esm.js	54.44 KB (+0.4% 🔺)
dist/browser.umd.js	54.83 KB (+0.39% 🔺)

This reverts commit 4d1cc22.

colebemis

Love where this is going ❤️ Amazing work, @siddharthkp!

Happy to move forward with what you have here. Left a few minor comments.

colebemis · 2021-11-10T00:30:22Z

src/theme-preval.js

@@ -1,5 +1,6 @@
 // @preval
 // This file needs to be a JavaScript file using CommonJS to be compatiable with preval
+// Cache bust: 2021-11-04 12:00:00 GMT (This file is cached by our deployment tooling, update this timestamp to rebuild this file)


There has to be a better way of doing this, but I did not spend more than 5 minutes researching vercel's caching mechanism. So, this date will probably live here until we find a different way 😇

src/utils/create-slots.tsx

colebemis · 2021-11-10T00:35:51Z

src/experiments.ts

@@ -0,0 +1,2 @@
+// Components
+export * from './ActionList2'


I like the idea of keeping experiments out of the main bundle. A couple of questions:

If we want to build experimental components in primer, should we update the definition of "experimental" in this document? https://primer.style/contribute/component-lifecycle#experimental (defines "experimental" as built outside of Primer)

Should we rename this file experimental to align with that document? ☝️

When we release breaking changes to experimental components, which semver number should we bump?

Hmm, there's a name conflict here for sure.

In our component lifecycle maturity, components with experimental maturity can still be in the main bundle.

What I want to signal here is - this is a "unreleased" component that is not part of the public api yet (not in main bundle). We don't recommend using it in production yet. You can still import it with explicitly for experimentation/feedback.

alternate names: unreleased, next, future, private, internal

When we release breaking changes to experimental components, which semver number should we bump?

As these are outside the public API, they don't get a major/minor version bump with the assumption that only prototypes/experiments are using them and not continuously maintained applications. (mostly safe because prototypes/sandboxes have a locked version in time)

Update: changed to unreleased.

(next was a close second, but that might get confused with npm tag primer/components@next. future is romantic, but i'd like to put long running things in this scope that might not make it to main bundle at all)

@pksjce thoughts for new Button?

(Useful to add, also added to PR description)

Why is ActionList2 in unreleased instead of replacing the current ActionList with a breaking change?

While I'm confident about the API in this PR (by testing it A LOT 😇), I'd like to "release" these changes along with their composable counterparts ActionMenu and SelectPanel, so that the memex team can catch up with breaking API for Menus together.

This also gives us a change to tweak and polish the underlying ActionList to be accessible. The change is not code aesthetics, it's a more mature group of components ✨

src/ActionList2/Visuals.tsx

src/ActionList2/List.tsx

src/ActionList2/Item.tsx

Co-authored-by: Cole Bemis <colebemis@github.com>

mperrotti · 2021-11-10T21:28:28Z

src/ActionList2/Item.tsx

+  /**
+   * Primary content for an Item
+   */
+  children?: React.ReactNode


Do we need to specifically define React.ReactNode?

I think so 🤔 Added this here because that's what we say in the docs

New ActionList component, bundled in @primer/components/unreleased Co-authored-by: Jonathan Fuchs <jfuchs@github.com> Co-authored-by: Cole Bemis <colebemis@github.com>

siddharthkp added 3 commits October 11, 2021 18:55

wip

0876cef

big chunk of parts

6866c09

leading visual width fix

4d9d014

Merge branch 'main' into action-list-2

9119eea

github-actions bot deployed to Preview October 15, 2021 01:43 View deployment

fix lint errors

e519b85

github-actions bot deployed to Preview October 15, 2021 01:47 View deployment

export list2 to see size diff

2224f84

github-actions bot deployed to Preview October 15, 2021 01:50 View deployment

nested stories

fed8f51

github-actions bot deployed to Preview October 15, 2021 13:42 View deployment

siddharthkp added 7 commits October 15, 2021 16:03

tiny cleanup

0630556

testing alternative slot implementation!

4d1cc22

Revert "testing alternative slot implementation!"

1c3ce94

This reverts commit 4d1cc22.

double render strategy

b07feef

add failing story for double render strategy

a6acb35

more failing cases with stateful children

4dae4c7

support children with side effect

ef8f3a2

github-actions bot deployed to Preview October 15, 2021 15:52 View deployment

siddharthkp added 2 commits October 15, 2021 18:21

one way to support dynamic description

cfd7e2d

register + unregister all slots

1748dcf

github-actions bot deployed to Preview October 15, 2021 16:58 View deployment

get the contents inside the effect

bcdff11

github-actions bot deployed to Preview October 15, 2021 17:13 View deployment

siddharthkp added 3 commits October 15, 2021 19:26

update tests

a07abf3

story doesnt have to be ugly

0fc658d

simplify with layout effect :)

2f3a8cd

github-actions bot deployed to Preview October 18, 2021 14:09 View deployment

simplify slot types

1432420

github-actions bot deployed to Preview November 8, 2021 11:58 View deployment

This was referenced Nov 8, 2021

ActionList: Correct semantics for Groups #1584

Closed

ActionList: Better label & description for Item #1585

Closed

add active color based on vini's feedback

9d14e5a

github-actions bot deployed to Preview November 8, 2021 17:48 View deployment

colebemis approved these changes Nov 10, 2021

View reviewed changes

LeadingVisualContainer should use VisualProps

71bc34e

Co-authored-by: Cole Bemis <colebemis@github.com>

github-actions bot deployed to Preview November 10, 2021 10:43 View deployment

accept html attributes on visual

bacd12c

github-actions bot deployed to Preview November 10, 2021 10:51 View deployment

rename propsSx to sxProp

859008e

github-actions bot deployed to Preview November 10, 2021 10:56 View deployment

rename experiments to unreleased

6d6ddf5

github-actions bot deployed to Preview November 10, 2021 11:08 View deployment

This was referenced Nov 10, 2021

ActionList: Use icon instead of input for multiple selection #1591

Closed

ActionList: Add keyboard accessibility #1593

Closed

ActionList: test stories for axe violations #1597

Closed

mperrotti approved these changes Nov 10, 2021

View reviewed changes

mperrotti reviewed Nov 10, 2021

View reviewed changes

mperrotti approved these changes Nov 10, 2021

View reviewed changes

Merge branch 'main' into action-list-2

3cfc4f7

github-actions bot deployed to Preview November 11, 2021 13:19 View deployment

siddharthkp merged commit 561c0bd into main Nov 11, 2021

siddharthkp deleted the action-list-2 branch November 11, 2021 13:32

primer-css mentioned this pull request Nov 11, 2021

Release Tracking #1547

Merged

siddharthkp mentioned this pull request Nov 11, 2021

ActionList: Better label & description for Item #1599

Merged

8 tasks

pksjce pushed a commit that referenced this pull request Nov 15, 2021

ActionList Composable API (#1517)

ee3e4c3

New ActionList component, bundled in @primer/components/unreleased Co-authored-by: Jonathan Fuchs <jfuchs@github.com> Co-authored-by: Cole Bemis <colebemis@github.com>

primer-css mentioned this pull request Nov 16, 2021

Release Tracking (next) #1612

Merged

radglob mentioned this pull request Nov 22, 2022

Fix issue where scroll containers nested inside of PageLayout.Pane would be hard to use on mobile. #2606

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ActionList Composable API #1517

ActionList Composable API #1517

siddharthkp commented Oct 15, 2021 •

edited

Loading

changeset-bot bot commented Oct 15, 2021 •

edited

Loading

github-actions bot commented Oct 15, 2021 •

edited

Loading

colebemis left a comment

colebemis Nov 10, 2021

siddharthkp Nov 10, 2021 •

edited

Loading

colebemis Nov 10, 2021

siddharthkp Nov 10, 2021 •

edited

Loading

siddharthkp Nov 10, 2021

siddharthkp Nov 10, 2021 •

edited

Loading

mperrotti Nov 10, 2021

siddharthkp Nov 11, 2021

ActionList Composable API #1517

ActionList Composable API #1517

Conversation

siddharthkp commented Oct 15, 2021 • edited Loading

API opinions worth noting:

changeset-bot bot commented Oct 15, 2021 • edited Loading

🦋 Changeset detected

github-actions bot commented Oct 15, 2021 • edited Loading

size-limit report 📦

colebemis left a comment

Choose a reason for hiding this comment

colebemis Nov 10, 2021

Choose a reason for hiding this comment

siddharthkp Nov 10, 2021 • edited Loading

Choose a reason for hiding this comment

colebemis Nov 10, 2021

Choose a reason for hiding this comment

siddharthkp Nov 10, 2021 • edited Loading

Choose a reason for hiding this comment

siddharthkp Nov 10, 2021

Choose a reason for hiding this comment

siddharthkp Nov 10, 2021 • edited Loading

Choose a reason for hiding this comment

mperrotti Nov 10, 2021

Choose a reason for hiding this comment

siddharthkp Nov 11, 2021

Choose a reason for hiding this comment

siddharthkp commented Oct 15, 2021 •

edited

Loading

changeset-bot bot commented Oct 15, 2021 •

edited

Loading

github-actions bot commented Oct 15, 2021 •

edited

Loading

siddharthkp Nov 10, 2021 •

edited

Loading

siddharthkp Nov 10, 2021 •

edited

Loading

siddharthkp Nov 10, 2021 •

edited

Loading