ActionList Component: Primer CSS Implementation #1657
Conversation
🦋 Changeset detectedLatest commit: d6dd77c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
| 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. |
There was a problem hiding this comment.
Is this description enough? I feel like I would just be repeating the interface guideline, but I could pull some snippets from it
docs/package.json
Outdated
| @@ -17,7 +17,7 @@ | |||
| "@primer/gatsby-theme-doctocat": "2.0.0", | |||
| "@primer/octicons": "16.1.0", | |||
| "@primer/octicons-react": "16.0.0", | |||
| "@primer/primitives": "6.0.0", | |||
| "@primer/primitives": "6.1.0-rc.9f1f534", | |||
There was a problem hiding this comment.
Temporary- will update with next release
src/actionlist/action-list-item.scss
Outdated
| @@ -0,0 +1,428 @@ | |||
| // stylelint-disable selector-max-specificity, max-nesting-depth, primer/spacing, no-duplicate-selectors | |||
There was a problem hiding this comment.
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 🤷
| // stylelint-disable selector-max-specificity, max-nesting-depth, primer/spacing, no-duplicate-selectors | ||
| @import './action-list-variables.scss'; | ||
|
|
||
| @mixin focusOutline { |
There was a problem hiding this comment.
We could pull out this mixin in the future if we roll out more global focus styles
| } | ||
| } | ||
|
|
||
| // place children on grid |
There was a problem hiding this comment.
This section creates the duplicate class selectors, but I think its nice having the grid defs pulled out together
| visibility: hidden; | ||
| opacity: 0; | ||
| transition: | ||
| visibility 0 linear $actionList-item-checkmark-transition-timing, |
There was a problem hiding this comment.
is this indentation correct?
There was a problem hiding this comment.
Thats what stylelint wants 🤷
docs/src/stories/components/ActionList/Accessibility.stories.mdx
Outdated
Show resolved
Hide resolved
| @@ -399,6 +403,9 @@ | |||
| .ActionList-item-label { | |||
| position: relative; // for pseudo dividers | |||
| 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; | |||
There was a problem hiding this comment.
line-height 20px
Conflicts: docs/package.json docs/yarn.lock package.json yarn.lock
ActionList CSS
The Primer CSS implementation of Action List
Interface guidelines
Figma documentation
React
File structure
src/actionlistulstylesliand contents styles (the biggie)lias divider stylesI 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/ActionListEach 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 ListCode review
.jsxstory files for review, they are for documentation and developmentTODO:
actionListItemvariables primitives#256action-list-helpers-temporary.scssand figure out what to do with proposed variable additions (should they be global or just used in action list)Fixes: https://github.com/github/primer/issues/4
/cc @primer/css-reviewers