primer / view_components

ViewComponents for the Primer Design System

Primer ViewComponents

ViewComponents for the Primer Design System.

Note: This library is under active pre-1.0 development. Breaking changes are likely in patch releases.

Documentation

Visit https://primer.style/components/ to view documentation.

License

The gem is available as open source under the terms of the MIT License.

view_components's Issues

Run accessibility audits automatically

Let's find a way to use https://github.com/dequelabs/axe-core

Some options:

• https://www.npmjs.com/package/@storybook/addon-a11y
• https://github.com/microsoft/accessibility-insights-action

Promote components that are ready to beta

• BorderBox
• Flash
• Spinner
• Octicon

flaky build failures on bundle

Things resolve on rebuild and errors tend to look like this:

## Backtrace

NoMethodError: undefined method `name' for "Gemfile":String
  /opt/hostedtoolcache/Ruby/2.7.2/x64/lib/ruby/gems/2.7.0/gems/bundler-1.17.3/lib/bundler/resolver/spec_group.rb:65:in `eql?'
  /opt/hostedtoolcache/Ruby/2.7.2/x64/lib/ruby/gems/2.7.0/gems/bundler-1.17.3/lib/bundler/vendor/molinillo/lib/molinillo/resolution.rb:607:in `block in create_conflict'
  /opt/hostedtoolcache/Ruby/2.7.2/x64/lib/ruby/gems/2.7.0/gems/bundler-1.17.3/lib/bundler/vendor/molinillo/lib/molinillo/resolution.rb:606:in `each'
  /opt/hostedtoolcache/Ruby/2.7.2/x64/lib/ruby/gems/2.7.0/gems/bundler-1.17.3/lib/bundler/vendor/molinillo/lib/molinillo/resolution.rb:606:in `create_conflict'
  /opt/hostedtoolcache/Ruby/2.7.2/x64/lib/ruby/gems/2.7.0/gems/bundler-1.17.3/lib/bundler/vendor/molinillo/lib/molinillo/resolution.rb:700:in `attempt_to_filter_existing_spec'
  /opt/hostedtoolcache/Ruby/2.7.2/x64/lib/ruby/gems/2.7.0/gems/bundler-1.17.3/lib/bundler/vendor/molinillo/lib/molinillo/resolution.rb:673:in `attempt_to_activate'

Promote HeadingComponent to beta

Should it exist at all? Should we jut use TextComponent with an argument?

If we want to keep it, let's decide what we need to do to promote it to beta.

Handle `nil` cases in storybook

While working on the label component: https://github.com/primer/view_components/blob/main/app/components/primer/label_component.rb

I noticed that there was an error when adding a storybook story. '

The label component has the following code:

SCHEME_MAPPINGS = {
      # gray
      gray: "Label--gray",
      dark_gray: "Label--gray-darker",

      # colored
      yellow: "Label--yellow",
      orange: "Label--orange",
      red: "Label--red",
      green: "Label--green",
      blue: "Label--blue",
      purple: "Label--purple",
      pink: "Label--pink",

      # Deprecated
      outline: "Label--outline",
      green_outline: "Label--outline-green",
    }.freeze
    SCHEME_OPTIONS = SCHEME_MAPPINGS.keys << nil

mainly this is the part that is giving us trouble: SCHEME_OPTIONS = SCHEME_MAPPINGS.keys << nil

However, the following method in the story is unable to handle nil cases:
select(:scheme, Primer::StoriesHelper.array_to_options(Primer::LabelComponent::SCHEME_MAPPINGS.keys), :blue)

That's because nil shows up as :nil --- a hash without a key.

cc/ @joelhawksley @aellispierce @jonspalmer

Update primer/css version

In dotcom we have "@primer/css": "15.2.0", and in the Rakefile to build the docs we have https://unpkg.com/@primer/css/dist/primer.css, which resolves to https://unpkg.com/@primer/[email protected]/dist/primer.css.

---

Ah, I see what's going on here.

The docs examples load Primer from a public CDN, not using the version we serve in dotcom.

We really need to update the PVC docs site to use the version of Primer CSS we're using on dotcom. Perhaps we could prioritize that ahead of this PR?

Originally posted by @joelhawksley in #269 (comment)

Support CSS class names like `color-text-success`

👋 hi there! I was working with some css classes today and received an error when I tried to apply classes: "color-text-success" to a Primer::OcticonComponent. I tried removing the classes keyword and using color: "text-success" but that got translated to text-text-success as the class. I tried a few other combinations but couldn't get that particular class to work out.

I'm wondering if:

• Could we add support for classes like color-text-success?
• Should something like color-text-success be allowed under the classes keyword?

Thanks so much for looking into this! ❤️

Fix Symbol / String Discrepancy in Storybook Params

@joelhawksley originally noticed this problem while updating the storybook for the flash and octicon component.

While working on the following PR: #79, I noticed I had the same issue.

Each time a selection is made to change the storybook from its default settings, the params render as a string rather than symbol ( Even though the keys in the hash are symbols), which prevents the user from making any sort of selection or change to the components in storybook. In order to work around this in a "hacky" way, we must add a .to_sym method to the variable that is being rendered in the component.

It would probably be better if we could figure out a way to do this without adding .to_sym to every selection variable in all the view components???

cc/ @joelhawksley @aellispierce @jonspalmer

Set up CI

Let's set up CI, using GitHub Actions.

Promote ButtonComponent to beta

TODO:

• The API of ButtonComponent.new(type: :button) seems like a smell we should resolve.

BorderBox demo seems to be broken?

The scrolling of this demo seems a bit broken.

Screen.Recording.2021-02-25.at.15.31.58.mov

Add a way to see component changes

Idea

We could use https://github.com/jonspalmer/view_component_storybook and modify the dummy app to serve a storybook that we could develop on

Promote StateComponent to beta

TODO:

We need to apply functional color changes before promoting.

We want to go from

     COLOR_DEFAULT => "",
      :green => "State--green",
      :red => "State--red",
      :purple => "State--purple"
    }.freeze

to

     COLOR_DEFAULT => "",
      :open  => "State--open",
      :closed => "State--closed",
      :merged => "State--merged"
    }.freeze

We should be able to use #135 as inspiration, which will allow for backwards compatibility.

Audit coverage of Primer CSS utilities

In order to promote Primer ViewComponents as our preferred solution for Rails UI, we need to make sure we have support for Primer CSS' utilities: https://primer.style/css/utilities.

For example, we do not have support for animations: https://primer.style/css/utilities/animations.

Parse `classes` argument with `class_names`

In an internal PR, we experimented with passing classes through class_names, allowing the class_names syntax to be used in a component's arguments.

I think this is a something we should standardize all Primer ViewComponents to have.

Add new label to docs site for components that require JS behaviors

Inspired by #282. We could do something like Requires JS

Expand support for passing in hex colors

In https://github.com/primer/view_components/pull/138/files#r527100055, @kevinsawicki rightly pointed out that our support for passing in color hex values for system arguments is inconsistent.

We should probably make this feature consistent across other color-related system arguments.

What do you think @emplums?

Support negative margin utilities

Primer css supports Negative Margins in the margin utilities. I've run into a situation a couple of times where I wanted to use a negative margin with a view component, but ml: "n1" doesn't work.

[TRACKING] Add documentation

Problem

Primer ViewComponents has little documentation. It's difficult for consumers of the library to know how to use it correctly.

Developers looking for documentation expect to find it at primer.style, but their search comes up empty.

Solution

Add a Primer ViewComponents section to primer.style.

TODO

• Test YARD documentation tool (done, see #64).
• Build out end-to-end proof of concept for generating markdown documentation from YARD, served by Doctocat.
• Write documentation for all components, using yardstick to ensure complete coverage.
• Ship documentation site on primer.style
• Add Status field for each component like we have in Primer CSS (New, Stable, etc)

Add CI build step to verify docs have been built

As we check in the generated documentation in the the repo, we should ensure the docs are up to date as part of our CI build.

Promote AvatarComponent to beta

TODO:

• there are usages in other codebases where we round the corners of the avatar. Let's determine if this use case should be built in.

FlexItemComponent doesn't support flex-basis

I found myself wanting to use flex-basis (namely, a % basis), and FlexItemComponent doesn't have anything exposed to pass it in alongside normal args (like flex_auto).

Ex:
<%= render(Primer::FlexItemComponent.new(flex_auto: true)) do %>
=>
<%= render(Primer::FlexItemComponent.new(flex_auto: true, flex_basis: "33%")) do %>

Note: It's also very common for people specifying flex in a way that "contains" stuff to use overflow: hidden to make sure that the flex boundaries are respected. I suspect that should be supported as well.

Build TimelineComponent

How to

Generally, we're trying to keep our ViewComponent syntax in line with the equivalent component in Primer Components. The Primer Components implementation of Timeline is quite complex, but possible to be implemented in ViewComponent. .

Resources

Primer CSS documentation
Primer Components documentation

Improve style arguments docs

Currently, the style arguments documentation is written from the perspective of migrating from Primer CSS.

While this is helpful for some users of the library, we aren't providing guidance for using style arguments from scratch. I think we should rewrite this documentation to better serve folks using the library for the first time.

Add support for selecting current link in menus

In order to aid adoption of our menu components, we should look into adding support for highlighting a link if it's the current page.

See https://github.slack.com/archives/C0ER2LCG2/p1604607507420600?thread_ts=1604595930.416600&cid=C0ER2LCG2

cc @elbrenn

Commands to compile demo

Hi, I was wondering if you can provide me an example of the arguments that you use when compiling the demo. I'd love to build this project locally, but my attempts have failed when trying to run the compile script.

<3

Add support for `border: true`

Oh! Looks like we need to add support for border: true somehow. Mind filing a separate issue?

Originally posted by @joelhawksley in #190 (comment)

Create helpers to render components

Rendering simple components such as Primer::HeadingComponent is verbose and not user friendly.
e.g:

<%= render Primer::HeadingComponent.new(tag: :h1) { "My Heading" } %>

We should provide helpers to simplify the API, something like:

<%= primer_heading(:h1, "My Heading") %>

Build out Storybook stories for all components

In order to provide a complete set of examples for Primer ViewComponents, we should write Storybook stories for every component.

We track the number of missing stories in a linter.

• Write stories for all components
• Link to stories from docs pages

cc @aellispierce @natashaU

Fix stuck CI when actions docs build is last commit

#257 caused a use case where PR's can get stuck if the docs build has the last commit since actions seems not to run. I would have expected https://github.com/primer/view_components/blob/main/.github/workflows/ci.yml#L3 to mean CI would run no matter where the commit came from.

Explore enforcing docs coverage via yardstick

There are projects like https://rubygems.org/gems/yardstick that could help us enforce documentation coverage.

We have some dev time logging that outputs components with missing docs, examples, or stories but maybe there is a way to handle this more pragmatically.

Standardize the language and voice we use in docs

[Tracking] PrimerViewComponents V1

This issue tracks everything we need to do in order to release v1 of primer_view_components. Feel free to comment your feedback on what makes sense and if we are missing anything.

Goals

• Documentation for all components
• Storybook for all components
• Upgrades to SlotsV2
• Paved path for components with behaviors #250
• ViewHelpers #260
• Promote X components to stable ?

Color-vision impairment, icon clarity

Issue

User report of issues distinguishing between PR states: open and closed; because of color indistinguishability, and non-distinct symbol shapes for open vs closed.

Comments on a thread.

I'm red-green colorblind but the only way it noticeably affects my life these days is with the icons on Github's notifications page. A merged PR an an open PR have different shapes and the merged PR is blue and the open PR is green --- very easy to distinguish.
But an open PR and a closed PR have identical shapes and are only distinguished by the open PR being green and the closed PR being red. Or so I'm told anyway, because I cannot for the life of me distinguish between the two. I end up needing to just open the link to the PR to check its status.

The closed PR icon has a tiny little tickmark in the icon I believe.
I had never realized they had different colors btw, and always wondered why GitHub wouldn't give them different colors. Thanks for clarifying that they actually did, just badly.
Tip for anyone using red and green for statuses: do like traffic lights: make the green a whiteish, blueish green and make the red tend a bit towards orange. And for the love of god use wildly different icon shapes!

Diagnosis / Checks

Color blindness web viewer, during development. Like Toptal or WebAIM.

Automated tests for color distinguishability? If it's worth the effort.

Checklist item in design system to ensure broad ease of use concerns. (e.g. here, symbol & color differences for icon-represented states.)

---

Note, I mistakenly issued this issue elsewhere first, because I didn't know where to issue it, nor where to find out where to issue it. Is this the right place?

Unable to use Primer::AvatarComponent with linked avatars inside AvatarStack

When working on https://github.com/github/github/pull/170198, I noticed unexpected behavior when using Primer::AvatarComponent to render linked avatars within an AvatarStack (specifically, "squished" avatars on hover):

According to the AvatarStack docs, when using AvatarStack with linked avatars, the "avatar" class should be applied to the anchor element only, not the image element:

You can also link individual avatars. To do this shift the avatar class over to the anchor:

<div class="AvatarStack AvatarStack--two">
  <div class="AvatarStack-body" aria-label="octocat and octocat">
    <a href="#" class="avatar">
      <img
        width="20"
        height="20"
        alt="@octocat"
        src="https://user-images.githubusercontent.com/octocat.png"
      />
    </a>
<!-- etc -->

• If the "avatar" class is applied to both the anchor element and the image element, you end up with the UI breakage shown above.
• If you apply the "avatar" class to the image element only, then the AvatarStack doesn't expand on hover.
• Primer::AvatarComponent always applies the "avatar" class to the image element. As far as I can tell, there's no way to override that default class.

Given these current constraints, there doesn't seem to be a way to use Primer::AvatarComponent with linked avatars inside AvatarStack.

@manuelpuyol mentioned that there are plans to implement a generic AvatarStack in the near future, which is great news 🎉 He also recommended opening this issue 😄

As a potential alternative, I wanted to throw out the idea of a "linked avatar" option for Primer::AvatarComponent, which would produce an image element wrapped in an anchor element similar to the markup above. Not sure if that's a lower lift than implementing a new component -- or something we even want to support -- but could be a solution for this particular blocker.

Fix errors on Rails 5.0.0 and reintroduce it to CI

https://github.com/primer/view_components/pull/5/checks?check_run_id=945770880

Feature: Avatar Stack Component

I'd like to make a PR for the avatar stack component. However, I see that currently, view_components has two Slotable modules,

• ViewComponent::Slotable
• ViewComponent::SlotableV2

If I am to make a PR, which module should I use?

flex component story is crashing

torybook | [./node_modules/@storybook/core/dist/server/common/polyfills.js] 120 bytes {vendors~main}
storybook | [./node_modules/@storybook/core/dist/server/preview/globals.js] 93 bytes {vendors~main}
storybook | [./node_modules/@storybook/core/node_modules/webpack/buildin/module.js] (webpack)/buildin/module.js 497 bytes {vendors~main}
storybook | [./node_modules/@storybook/server/dist/client/index.js] 1.21 KiB {vendors~main}
storybook | [./node_modules/airbnb-js-shims/index.js] 40 bytes {vendors~main}
storybook | [./node_modules/core-js/features/symbol/index.js] 359 bytes {vendors~main}
storybook | [./node_modules/webpack-hot-middleware/client.js?reload=true&quiet=false&noInfo=undefined] 7.68 KiB {vendors~main}
storybook |     + 716 hidden modules
storybook |
storybook | WARNING in ../stories/primer/flex_component.stories.json
storybook | Module build failed (from ./node_modules/@storybook/server/dist/server/loader.js):
storybook | TypeError: Cannot convert undefined or null to object
storybook |     at Function.keys (<anonymous>)
storybook |     at stringifyObject (/Users/srt32/src/github.com/primer/view_components/demo/node_modules/@storybook/server/dist/lib/compiler/stringifier.js:130:16)
storybook |     at /Users/srt32/src/github.com/primer/view_components/demo/node_modules/@storybook/server/dist/lib/compiler/stringifier.js:120:14
storybook |     at Array.map (<anonymous>)
storybook |     at stringifyObject (/Users/srt32/src/github.com/primer/view_components/demo/node_modules/@storybook/server/dist/lib/compiler/stringifier.js:119:31)
storybook |     at /Users/srt32/src/github.com/primer/view_components/demo/node_modules/@storybook/server/dist/lib/compiler/stringifier.js:132:21
storybook |     at Array.map (<anonymous>)
storybook |     at stringifyObject (/Users/srt32/src/github.com/primer/view_components/demo/node_modules/@storybook/server/dist/lib/compiler/stringifier.js:131:47)
storybook |     at /Users/srt32/src/github.com/primer/view_components/demo/node_modules/@storybook/server/dist/lib/compiler/stringifier.js:132:21
storybook |     at Array.map (<anonymous>)
storybook |     at stringifyObject (/Users/srt32/src/github.com/primer/view_components/demo/node_modules/@storybook/server/dist/lib/compiler/stringifier.js:131:47)
storybook |     at /Users/srt32/src/github.com/primer/view_components/demo/node_modules/@storybook/server/dist/lib/compiler/stringifier.js:132:21
storybook |     at Array.map (<anonymous>)
storybook |     at stringifyObject (/Users/srt32/src/github.com/primer/view_components/demo/node_modules/@storybook/server/dist/lib/compiler/stringifier.js:131:47)
storybook |     at /Users/srt32/src/github.com/primer/view_components/demo/node_modules/@storybook/server/dist/lib/compiler/stringifier.js:181:73
storybook |     at Array.forEach (<anonymous>)
storybook |  @ \.)(?=.)[^/]*?\.stories\.json)$ (/Users/srt32/src/github.com/primer/view_components sync ^\.(?:(?:^|\/|(?:(?:(?!(?:^|\/)\.).)*?)\/)(?!\.)(?=.)[^/]*?\.stories\.json)$) ./stories/primer/flex_component.stories.json
storybook |  @ ./.storybook/generated-stories-entry.js

Deprecate Flex and FlexItem components in favor of BoxComponent

Let's choose to deprecate them in favor of BoxComponent usage or let's figure out what we need to do to promote them to beta.

Promote LabelComponent to beta

TODO:

• We need to remove the deprecated color options before promoting.

Automate building docs on PR's

We can use https://github.com/github/catalyst/blob/main/.github/workflows/docs-build.yml as inspiration.

We can also dump statuses as well.

Setting label background

Trying to replicate labels with background
https://primer.style/css/components/labels#issue-labels

I used this in erb:

render Primer::LabelComponent.new(scheme: :red, color: :white, bg: :red)

which shows this:

The reason seems to be the "initial!important" override:

UnderlineNavComponent slots and responsive support

We're wanting to package up the overflow functionality that the repository underline nav has and include it in the Primer view_component.

The current state of this code is that it uses Primer::UnderlineNavComponent but content areas to loop through a list of links once and re-loop through them again to render matching links in the main nav and dropdown nav. Ideally we should be able to provide item slots to the component and the responsive dropdown is generated automatically.

Here's what we'll need to do to get this support.

• Convert UnderlineNavComponent to using the slots api
• By default render the overflow menu dropdown with the option to turn it off.
• Package up the js responsible for calculating when to overflow an item?

Build SelectMenuComponent

How to

Generally, we're trying to keep our ViewComponent syntax in line with the equivalent component in Primer Components. The Primer Components implementation of SelectMenu is quite complex, but possible to be implemented in ViewComponent.

Resources

Primer CSS documentation
Primer Components documentation

Classify breaks when empty string is passed to `color:`

problem is here:

view_components/lib/primer/classify.rb

Line 206 in 386ac0c

char_code = val[-1].ord

Deploy demo app storybook

We should deploy the demo app storybook so people could check components without having to clone/read the code

Measure and reduce the number of allocations made by Primer::Classify

When investigating a page's performance related to allocations, I found that Primer::Classify was performing a significant number of allocations.

The places making the most allocations were:

• Primer::Classify#extract_class
• Hash#except
• Line 187 of Primer::Classify#extract_class

We should try to measure, then reduce the number of allocations made in that class since it can be a hot path for primer components.

Should we drop support for Ruby 2.4?

2.4 is EOL as of 2020-03-31 and requires some maintenance to maintain. Should we drop it from the build matrix?

How to document SlotV2 arguments

Related: #185 (comment)

We need to document arguments for our component's slots. Currently we document directly on their Slot class, but with the introduction of SlotV2 the syntax has changed and we need to decide how to document them.

Require branches to be synced with main before merging

This will prevent outdated branches (which could fail CI later) to be merged and break main.

Example:
a653a85