ActionList Component: Primer CSS Implementation by langermank · Pull Request #1657 · primer/css

langermank · 2021-10-04T21:56:31Z

ActionList CSS

The Primer CSS implementation of Action List
Interface guidelines
Figma documentation
React

File structure
src/actionlist

action-list.scss: ul styles
action-list-item.scss: li and contents styles (the biggie)
action-list-item-divider.scss: li as divider styles

I broke the CSS into separate files by "component" level because I've heard some talk of encapsulating styles. I can move everything into one file if that's standard.

docs/src/stories/components/ActionList

Each individual component level has its own story which creates a "template". I'm then using that template SomeTemplate.bind({}) in feature/pattern stories to showcase every possible variation of Action List

Code review

Feel free to ignore .jsx story files for review, they are for documentation and development
Looking for feedback on the following:
- Nesting
- BEM (how'd I do?)
- Performance (any red flags?)
- Support (did I include any new CSS properties we don't support?)
- Visual review when Storybook is deployed

TODO:

Pull in primitive update + remove custom variables Add actionListItem variables primitives#256
Reconcile action-list-helpers-temporary.scss and figure out what to do with proposed variable additions (should they be global or just used in action list)
Deploy Storybook for stress testing
Finish up docs
Upgrade primitives + fix disabled color var Release Tracking primitives#263

Fixes: https://github.com/github/primer/issues/4

/cc @primer/css-reviewers

changeset-bot · 2021-10-04T21:56:33Z

🦋 Changeset detected

Latest commit: d6dd77c

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

This PR includes changesets to release 1 package

Name	Type
@primer/css	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

…nlist

langermank · 2021-10-28T20:01:36Z

+storybook: https://primer.style/css/storybook/?path=/story/components-actionlist-actionlistitem--playground
+---
+
+Reference the [Action list interface guidelines](https://primer.style/design/components/action-list) for details on where and how to use Action List.


Is this description enough? I feel like I would just be repeating the interface guideline, but I could pull some snippets from it

langermank · 2021-10-28T20:02:11Z

 "@primer/octicons": "16.1.0",
 "@primer/octicons-react": "16.0.0",
- "@primer/primitives": "6.0.0",
+ "@primer/primitives": "6.1.0-rc.9f1f534",


Temporary- will update with next release

langermank · 2021-10-28T20:05:45Z

@@ -0,0 +1,428 @@
+// stylelint-disable selector-max-specificity, max-nesting-depth, primer/spacing, no-duplicate-selectors


I'll review where I'm super duper nesting and using a lot of specificity.. but I think I need to with this component. Its designed to be pretty smart and recognize whats going on in the dom/make decisions, but maybe its too expensive 🤷

langermank · 2021-10-28T20:07:07Z

+// stylelint-disable selector-max-specificity, max-nesting-depth, primer/spacing, no-duplicate-selectors
+@import './action-list-variables.scss';
+
+@mixin focusOutline {


We could pull out this mixin in the future if we roll out more global focus styles

langermank · 2021-10-28T20:10:11Z

+ }
+}
+
+// place children on grid


This section creates the duplicate class selectors, but I think its nice having the grid defs pulled out together

vdepizzol · 2021-10-28T22:21:20Z

+ visibility: hidden;
+ opacity: 0;
+ transition:
+ visibility 0 linear $actionList-item-checkmark-transition-timing,


is this indentation correct?

Thats what stylelint wants 🤷

langermank · 2021-10-29T22:49:15Z

 font-weight: $font-weight-normal;
+ // we need a strict value here for grid alignment
+ // stylelint-disable-next-line primer/typography
+ line-height: $actionList-item-label-line-height;


line-height 20px

Conflicts: docs/package.json docs/yarn.lock package.json yarn.lock

jonrohan

This is looking really great 🌈

Approving! 👍🏻 Feel free to merge, whenever you feel like the API/naming works with what we're implementing in dotcom.

add storybook, start actionlist
1f7d08e

story composition, some katie edits
5eb8831

github-actionsBothad a problem deploying to Preview October 4, 2021 22:51 Failure

Merge branch 'main' ofhttps://github.com/primer/cssinto actionlist
20c18e9

github-actionsBothad a problem deploying to Preview October 5, 2021 16:24 Failure

langermank added 6 commits October 5, 2021 10:55

auto grid test
5867862

one grid option
26f4464

you win flexbox, you win
b39f941

back on that grid grind
7734667

story for each item context
9c70c46

import stories
a7967fe

github-actionsBothad a problem deploying to Preview October 5, 2021 22:59 Failure

add CSS friendly args for controls
a375fef

github-actionsBothad a problem deploying to Preview October 6, 2021 03:32 Failure

vdepizzol mentioned this pull request Oct 6, 2021
Make clear ActionList.Item's trailingIcon can be a component primer/react#1435
Closed

more conditionals
6fcbe26

github-actionsBothad a problem deploying to Preview October 6, 2021 18:44 Failure

push
f98f7ce

github-actionsBothad a problem deploying to Preview October 6, 2021 21:29 Failure

fix import
32bbc68

github-actionsBothad a problem deploying to Preview October 6, 2021 21:31 Failure

langermank added 3 commits October 6, 2021 14:47

clean up deps
e7e5eac

fix borders + hover state
bb73c0f

add navigational to story
8f4c3e5

langermank mentioned this pull request Oct 7, 2021
Storybook for Primer CSS #1660
Closed

langermank added 4 commits October 7, 2021 15:39

primitive-ize
a518486

break out files
71ef4a3

a11y fix
138b596

list stories
5fc9181

langermank added 2 commits October 28, 2021 11:57

change calc function
5b28b0b

Merge branch 'main' ofhttps://github.com/primer/cssinto actionlist
d358672

github-actionsBotdeployed to Storybook Preview October 28, 2021 19:00 View deployment

langermank added 2 commits October 28, 2021 12:39

remove fix script
6770fe7

lint, docs fixes
32a1e12

github-actionsBotdeployed to Storybook Preview October 28, 2021 19:57 View deployment

actions-userand others added 3 commits October 28, 2021 19:58

Stylelint auto-fixes
7ac2cca

guessing the storybook link for docs
6a68c22

Merge branch 'actionlist' ofhttps://github.com/primer/cssinto actio…
e442b62
…nlist

github-actionsBotdeployed to Storybook Preview October 28, 2021 19:58 View deployment

langermank commented Oct 28, 2021
View reviewed changes

vdepizzol reviewed Oct 28, 2021
View reviewed changes

langermankand others added 7 commits October 29, 2021 09:57

testing scss imports
7c793ed

Merge branch 'main' ofhttps://github.com/primer/cssinto actionlist
e421409

lint
e7344cb

default background
27d7ac4

handle padding for sizes
c67690e

remove comments
a7ac5e1

Stylelint auto-fixes
8474859

langermank commented Oct 29, 2021
View reviewed changes

Comment threadsrc/actionlist/action-list-item.scss

langermank commented Oct 29, 2021
View reviewed changes

langermankand others added 3 commits November 1, 2021 11:05

Merge branch 'main' ofhttps://github.com/primer/cssinto actionlist
a1eb1ba

Merge branch 'main' into actionlist
ea5f71b
Conflicts: docs/package.json docs/yarn.lock package.json yarn.lock

Create poor-walls-occur.md
73090da

github-actionsBot added the minor release label Nov 2, 2021

Update poor-walls-occur.md
d6dd77c

jonrohan approved these changes Nov 2, 2021
View reviewed changes

langermank merged commit e754300 into mainNov 2, 2021

langermank deleted the actionlist branch November 2, 2021 15:37

primer-css mentioned this pull request Nov 2, 2021
Release Tracking #1733
Merged

		@@ -0,0 +1,428 @@
		// stylelint-disable selector-max-specificity, max-nesting-depth, primer/spacing, no-duplicate-selectors

Uh oh!

Conversation

langermank commented Oct 4, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ActionList CSS

Code review

Uh oh!

changeset-botBot commented Oct 4, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonrohan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

langermank commented Oct 4, 2021•
edited
Loading

changeset-botBot commented Oct 4, 2021•
edited
Loading