primer / brand

React components and Primitives for GitHub marketing websites

Primer CSS

The CSS implementation of GitHub's Primer Design System

Documentation

⚠️ The documentation of this repo is not maintained anymore. Please raise any documentation-specific pull requests in primer.style/design

Our documentation site lives at primer.style/css. You'll be able to find detailed documentation on getting started, all of the components, our theme, our principles, and more.

Install

This repository is distributed with npm. After installing npm, you can install @primer/css with this command:

npm install --save @primer/css

Usage

The included source files are written in Sass using SCSS syntax. After installing with npm, you can add your project's node_modules directory to your Sass include paths (AKA load paths in Ruby), then import it like this:

@import "@primer/css/index.scss";

You can import individual Primer modules directly from the @primer/css package:

@import "@primer/css/core/index.scss";
@import "@primer/css/product/index.scss";
@import "@primer/css/marketing/index.scss";

Development

See DEVELOP.md for development docs.

Releasing (for GitHub staff)

You can find docs about our release process in RELEASING.md.

License

MIT © GitHub

brand's Issues

Daniel Burka Photo Test

MinimalFooter: allow buttons

We're looking to replace the custom Footer we have on some pages of https://resources.github.com/ with MinimalFooter. To do this, we need the option to allow button as alternative to MinimalFooter.Link.

Example: as you can see at the bottom of https://resources.github.com/enterprise/trial/, we have a button in the footer for "Manage cookies", which opens an overlay to manage cookie settings. The styling should be the same as the text link.

🐛 [BUG] - Can't set a custom logo in SubdomainNavBar

Describe the bug

I would like to set my own logo in SubdomainNavBar component, but there is no setting for that.

Reproduction steps

I noticed there is a `logoHref` prop on SubdomainNavBar component but no way to change the logo.

Expected behavior

If I can change the url that logo of SubdomainNavBar component is pointing to, I'd expect it to also support changing the logo itself.

Screenshots

![DESCRIPTION](LINK.png)

Browsers

No response

OS

No response

🐛 [BUG] - Unable to generate VRT snapshots through Docker on Apple silicon Macs

Describe the bug

On newer M1 chips, we are seeing a compatibility error generating visual regression testing snapshots.

Visual regression testing snapshots are currently generated through Playwright using their official Docker image. This lets us create snapshots either on the server or locally on macbooks.

On Intel-based devices like macbooks, the image executes correctly.

From @JoshBowdenConcepts...

++ arch
+ [[ aarch64 == \a\a\r\c\h\6\4 ]]
+ echo 'ERROR: not supported on Linux Arm64'
+ exit 1
ERROR: not supported on Linux Arm64
Failed to install browsers
Error: Failed to install chrome
npm ERR! Lifecycle script `test:visual:update-snapshots` failed with error: 
npm ERR! Error: command failed 
npm ERR!   in workspace: @primer/[email protected] 
npm ERR!   at location: /Users/jbowden/Documents/brand/packages/e2e 

Running in emulation mode by passing --platform linux/amd64 also doesn't work.

Reproduction steps

> **Warning**
> Only reproducible on M1 Apple chips

1. Clone repo
2. Checkout `main`
3. `cd packages/e2e && npm run test:visual:update-snapshots`
4. See error

Expected behavior

1. Clone repo
2. Checkout `main`
3. `cd packages/e2e && npm run test:visual:update-snapshots`
4. Passes without error

Screenshots

n/a (see main description)

Browsers

No response

OS

Mac

[BUG] Test if this issue is created in Primer backlog

Describe the bug

Does this issue end up in Primer backlog inbox?

Add presets for granular control over font widths, weight and slant in Heading component

With Mona Sans, we can now offer granular control over typographic visual appearance.

E.g.

• Could Heading support some 'presets' or official variants at a system / library level?
• Should we provide a clean API to allow users to experiment more easily with the font?
• What is the recommended approach for leveraging in Primer Brand?

Form components: conflict with id and name attributes for input

The current API for FormControl relies on the id field to assign the necessary name attribute for input tags. I would expect to be able to assign a custom name attribute to any input, regardless of what id was defined for the FormControl.

This creates an autogenerated id and name

<FormControl>
  <FormControl.Label>Name</FormControl.Label>
  <TextInput />
</FormControl>

This creates an autogenerated id and name, ignores the custom name set on the input

<FormControl>
  <FormControl.Label>Name</FormControl.Label>
  <TextInput name="name" />
</FormControl>

This sets id and name to the value name

<FormControl id="name">
  <FormControl.Label>Name</FormControl.Label>
  <TextInput />
</FormControl>

Next.js Link component incompatible with Hero CTAs

Problem statement

Next.js encourage using the Link component declaratively to wrap any <a> tags.

E.g.

<Link href="/about">
  <a>About Us</a>
</Link>

The Hero doesn't use a composable API, which makes composition of Next.js link impossible.

<Hero
  heading="..."
  description="..."
  primaryAction={{
    text: 'Primary action',
    href: '#',
  }}
  secondaryAction={{
    text: 'Secondary action',
    href: '#',
  }}
  align="..."
/>

Suggestion

Rewrite the API for consistency with other composable API's.

<Hero align="...">
  <Hero.Heading></Hero.Heading>
  <Hero.Description></Hero.Description>
  <Hero.Description></Hero.Description>
  <Link href="#">
    <Hero.PrimaryAction></Hero.PrimaryAction>
  </Link>
  <Link href="#">
    <Hero.SecondaryAction></Hero.SecondaryAction>
  </Link>
</Hero>

References

Next.js Link component docs

🐛 [BUG] - CTABanner doesn't forward className

Describe the bug

CTABanner doesn't currently forward the className prop. This makes it harder to apply custom styling during integration.

Line 56 in CTABanner.tsx will need to manually pass a a destructured className as the final argument.

Reproduction steps

1. Go to the docs page https://primer.style/brand/components/CTABanner
2. Pass the `className` prop to the CTABanner.
3. Open inspector and verify that class doesn't appear on the root element

Expected behavior

Class should be forwarded and appended to the end of all other existing class names

Screenshots

![primer docs snippet showing test class not being rendered into dom](https://user-images.githubusercontent.com/13340707/217572388-c1a9c8eb-0e4a-4876-ae27-aa2cf7b28062.png)

Browsers

No response

OS

No response

🐛 [BUG] - Textarea doesn't honor "required" attribute

Describe the bug

The following snippet:

    <FormControl required>
      <FormControl.Label>Example</FormControl.Label>
      <Textarea />
    </FormControl>

Creates a textarea with * appended to the label, so it appears required. However, it's not required to submit the form.

Reproduction steps

"required" attribute is missing from the html.

Expected behavior

The user is unable to submit the form unless the required textarea has a value.

Screenshots

^ above ^

Browsers

Chrome

OS

Mac

Allow custom Headings for River and FAQ component

River.Content does not accept a Heading with as="h2". River automatically applies as="h3" by default.

Using River and FAQ automatically uses h3 headings. Semantically, that doesn't always make sense, e.g. on https://accelerator.github.com the heading structure now goes from h1 to h3. Would be great to be able to customize the heading to h2 etc. when needed.

Actually, I would expect any component that uses Heading to be able to accept any parameters that Heading accepts.

cc @aguevara23

[Figma] Refactor FAQ

• Add responsive variants using the grid, like the MinimalFooter
• Add documentation to replace the existing notes.

[MinimalFooter] Add Instagram to `socialLinks`

This came up in https://github.com/github/githubuniverse.com/issues/1711#issuecomment-1539633063. The request would be to add an instagram option to the socialLinks and link to http://instagram.com/github.

Apply `text-decoration:none` also to the `:hover` state

Components that may also serve as links (such as Button or InlineLink) have text-decoration:none rules only applied to their normal state. This can create conflicts when sites use a stylesheet that explicitly sets text-decoration:underline for a:hover (for example, for accessibility reasons).

I think adding text-decoration:none to the :hover state would fix the problem.

[Accessibility] Add tooltip to `MinimalFooter` icons

There was a recent accessibility audit for https://githubuniverse.com.

Expected result
On the footer section, the tooltips should be defined for the twitter, github, linkedin, youtube and facebook controls.

The MinimalFooter component currently has the visually-hidden text for screen readers. But I assume we would also need something for mouse and keyboard users that might not know what the icon (e.g. twitter bird) means. In the audit it says adding a "tooltip", but not sure if it needs to be a Tooltip like we have in Primer React? It's still marked as "Not reviewed for accessibility". Or if adding a title attribute that the browser shows as a tooltip would be enough? Maybe something to get clarity with the #accessibility team first.

Improvements to Heading component sizing

Background

The Heading component currently accepts an as prop with valid values of h1, h2, h3, h4, h5 and h6.

The same values are then applied to the rendered output. E.g. as="h2" will render a <h2>.

Default styling is provided for each heading size, based on the the type scale for headings. E.g. h2 maps to 800 on the type scale.

The problem space

The tight-coupling of semantic tags to text sizing creates ergonomic challenges consumer side, and is too rigid for app-side use.

Where granular control is often required around heading sizing, the only option available to escape the defaults are to pass a custom CSS class.

E.g. in #100, where example layouts have been provided in Storybook but incorrect as sizing is used to meet the visual needs of the examples.

Proposed solution

Add escape hatches to the Heading API, in the form of a size prop. This would override the default sizing and allow more granular control over the in-browser visuals.

E.g.

<Heading as="h2" size="4">A h2 sized to look like a h4</Heading>

This API would avoid a breaking change by maintaining the existing guardrails and system defaults. It would however, offer a sufficient escape-hatch for app developers that continues to align with our type scale.

Remove visual.spec.d.ts files from published packages

We are unintentionally publishing Component.visual.spec.d.ts files in the published packages. These aren't relevant to our users. We should consider removing them, particularly as they require manual maintenance in the monolith.

E.g. https://unpkg.com/browse/@primer/[email protected]/lib/Accordion/

🐛 [BUG] - Button box shadow problem during OS alerts

The Button has a problem with box-shadows in focus outlines.

Example:

Screen.Recording.2023-03-09.at.17.42.40.mov

Steps to reproduce:

1. Go to https://galaxy.github.com/register
2. Complete all fields up to day selection
3. Press register
4. Observe validation appear
5. Observe box shadow issue on form

🐛 [BUG] - Too strict validations prevent acceptance of valid outputs

Describe the bug

Hey folks 👋, one major use case for us is using Primer Brand components with data coming from a CMS. That works smoothly for several components but presents some challenges with a few of them. For example, the River.Content component has a strict validation in the kind of children it may contain (according to the docs, it does only support Text, Heading, and Link). That prevents us from using components such as ReactMarkdown to parse incoming Markdown data and transform it into the same Text and Link components that the River.Content component expects to contain.

Here is a link to a Codepen showing this behavior. As shown there, the ReactMarkdown component will return a Text element with an InlineLink inside, something that River.Content should be able to handle. Instead, it ignores the ReactMarkdown element entirely and shows a blank space.

Do you have any guidance about how to proceed in such cases?

Reproduction steps

https://codepen.io/sergioalvz/full/bGmaqBx

Expected behavior

The `River.Content` component renders the result of evaluating the `ReactMarkdown` element.

Screenshots

No response

Browsers

No response

OS

No response

Set list-style-position to inside

Can we ensure ul and ol use list-style-position: inside; in the FAQs?

Before

After

[Accessibility] Improve `SubdomainNavBar` keyboard navigation

This came up in the Universe 2023 audit:

Users navigating in resize mode will get confused as the focus order is illogical as in forward navigation hamburger remains expanded even when focus moves out of it.

As far as I understood it..

• Keyboard focus should "loop back" to the first focusable control of the menu (I guess the close X button?) and not go to the next focusable item on the page while keeping the menu open.
• Menu should close when pressing the esc key.

Note: The report mentions "200% zoom", but I think that's unrelated and only needed to make the hamburger menu appear. Alternative is to just resize the browser window.

🐛 [BUG] - wrong doc link in README

Describe the bug

This is not related to an issue with brand but an issue in the README.

At the top of the README there is a link called primer.style/brand this link doesn't point to primer.style/brand but to primer.github.io/brand (this way works).

After there is a second link called primer.style/brand which point to primer style/brand (right) but primer.style/brand return a 404 error

Reproduction steps

Go to primer.style/brand and you will get a 404 error

Go to primer.github.io/brand and it will work

Expected behavior

Go to primer.style/brand and get the doc like on primer.github.io/brand

🐛 [BUG] - Grid Column with responsive map is misbehaving

Describe the bug

When using the Grid component, some combinations of column spans are not working when set with a responsive map.

Reproduction steps

This works:

<Grid enableOverlay>
  <Grid.Column span={9}>
    ...
  </Grid.Column>
  <Grid.Column span={3}>
    ...
  </Grid.Column>
</Grid>

This does not work:

<Grid enableOverlay>
  <Grid.Column span={{medium: 9}}>
    ...
  </Grid.Column>
  <Grid.Column span={{medium: 3}}>
    ...
  </Grid.Column>
</Grid>

Expected behavior

When using a responsive map to set the columns spans, the columns should stack as expected.

Screenshots

No response

Browsers

Chrome

OS

Mac

[Proposal] Full-width button

It would be useful to have a full-width prop to the Button component.

[Accessibility] Add a "Skip To Content" button to `SubdomainNavBar`

Note: This is not really urgent, but still thought it might be a "nice to have" at some point, since I assume we will run into it for every site/page we'll build.

---

This came up in https://github.com/github/accessibility-audits/issues/5220, where we got asked to add a "Skip To Content" button that is shown when a user uses the tab key to navigate and wants to skip having to go through all the options of the nav (SubdomainNavBar).

Here is the implementation for Universe. The button could be the first item in the SubdomainNavBar, but not sure about adding an id="start-of-content" that the button links to. For Universe we added it on the following <main> element. But ideally that should happen automatically without the need to remember it. Hmm.. Maybe since it's essentially just a "skip the nav", we might can also add an empty <div id="start-of-content" at the very end of SubdomainNavBar? 🤔

[Figma] Add width and letter spacing configurability to Heading

• Update text style presets to include condensed, normal, and expanded sizes (stretch) and none, normal, and condensed letter spacings.
• Add stretch and letterSpacing props to Heading component.

Component API consistency

The API for Hero, River and FAQ components is very different at the moment. We should strive for a common way of doing things.

It's a mix of this.

<Component heading="..." />

<Component>
  <Heading>...</Heading>
</Component>

<Component>
  <Component.Heading>...</Component.Heading>
</Component>

Current Hero

<Hero
  heading="This is my super sweet hero heading"
  description="Lorem ipsum dolor sit amet, consectetur adipiscing elit. In sapien sit ullamcorper id. Aliquam luctus sed turpis felis nam pulvinar risus elementum."
  primaryAction={{
    text: 'Primary action',
    href: '#',
  }}
  secondaryAction={{
    text: 'Secondary action',
    href: '#',
  }}
  align="center"
/>

Current River

They can be composed in any order, but their rendered output will always be in the in the predetermined order.

This API suggests to me that I could pass in more than one <Text /> component (or other), but actually only the first one is accepted. If we control the allowed number of components and the order, there is no reason to declare River.Content.

<River>
  <River.Visual>
    <img
      src="https://via.placeholder.com/600x400/f5f5f5/f5f5f5.png"
      alt="placeholder, blank area with an off-white background color"
    />
  </River.Visual>
  <River.Content>
    <Heading>Heading</Heading>
    <Text>
      Lorem ipsum dolor sit amet, consectetur adipiscing elit. In sapien sit
      ullamcorper id. Aliquam luctus sed turpis felis nam pulvinar risus
      elementum.
    </Text>
    <Link href="#">Call to action</Link>
  </River.Content>
</River>

Current FAQ

<FAQ>
  <FAQ.Heading>Frequently asked questions</FAQ.Heading>
  <FAQ.Item>
    <FAQ.Question>
      What&apos;s included in the GitHub for Startups offer?
    </FAQ.Question>
    <FAQ.Answer>
      <p>
        All GitHub for Startups companies receive up to 20 seats of GitHub
        Enterprise for free for year one and 50% off year two. Learn more
        about the features and capabilities of GitHub Enterprise{' '}
        <InlineLink
          href="https://www.github.com"
          target="_blank"
          rel="noreferrer"
        >
          here
        </InlineLink>
        .
      </p>
    </FAQ.Answer>
  </FAQ.Item>
</FAQ>

Publicize undocumented components

primer.style/brand documentation currently shows a handful of cherry-picked UI patterns, rather than the entire inventory of components available through the npm package.

List of undocumented components at time of issue creation:

• Button
• InlineLink
• Accordion
• Link
• ThemeProvider (merged into theming, but could benefit from separate docs for the React component API)
• ExpandableArrow
• hooks
  • useKeyboardEscape
  • useOnClickOutside
  • useIsomorphicEffect
• Page layout examples
  • Kitchen sink

Proposal

• Review components listed above and decide which ones (if any) warrant separate doc pages.
• Publish new doc pages

Why do this?

• Increase adoption of Primer Brand library by accurately documenting existing inventory.
• Storybook is being used as a source of truth and shopfront right now as docs examples are sparse.

Questions

• Should hooks be documented?
• Categorisation of components and impact on discoverability.. should Buttons and InlineLink go under Call to Actions or Components?

🐛 [BUG - React] - FormControl.Validation layout problems with Checkbox

The following snippet:

<FormControl validationStatus='error'>
  <FormControl.Label>Day</FormControl.Label>
  <Checkbox name="day" value="" />
  <FormControl.Validation className="test">Please select at least one day</FormControl.Validation>
</FormControl>

Produces this:

This happens because of row-reverse flex layout method.

Expected behavior

• Validation appears below the input and label per the usual validation layout: https://primer.style/brand/components/FormControl#validation

Suggested new default props

The request

The className prop, as I understand it, was added so that a consumer can safely assume that every component will take it. I feel as though we should also always allow:

• ref (React Ref)
• id (string)

Reasoning / thought process

• ID: Jump links or other unique
• ref: allowing this allows consumers to avoid using document.querySelector

I'm happy to create a PR to add a base type. Something like:

type BaseComponent = {
  className?: string
  id?: string
  ref?: RefObject<T>
}

[Accessibility] Visual name and aria-label are different for 'GitHub' control

As described in https://github.com/github/accessibility-audits/issues/5252

Users who rely on screen readers will get impacted, if the visual name and aria-label are different for a control. The users will not get the exact information about the control and may miss the functionality.

🐛 [BUG] - Button focus animation

Describe the bug

The Button component in the recent 0.12.1 release has a visual regression, whereby the focus outline animation is transitioning very slowly into position.

Reproduction steps

1. Go to https://primer.style/brand/components/CTABanner
2. Change live code snippet to dark color mode
3. Press the primary button
4. See the focus bug

Expected behavior

Pressing the Button should not produce a transition animation for the focus outline

Screenshots

Screen.Recording.2023-02-16.at.16.18.21.mov

Browsers

Reproducible in Chrome

OS

No response

Feature Request / Idea - View Grid columns for development

What is proposed:

I propose adding a "toggle-able" visual layer to our development environments. This layer would display the Primer grid columns as an overlay overtop our markup, similar to how Figma allows us to toggle the grid overlay inside of our designs.

This would utilize our tokens and would be a visual aide in development.

Reasoning

Often, particularly when grids are nested, it's easy to veer from the design without realizing it. A margin or padding in a parent element may affect the alignment of an element within the intended grid.

I believe having a visual indicator inside our design system would:

• help the speed and ease development as developers could turn on the grid to quickly and easily see how closely the markup matches the design (more easily comparing apples to apples) by placing items precisely on the grid
• improve the final product: differences between design and dev should be easier to spot (Less of Something looks off but I don't know what)
• Help developers see nested grids work in relation to the overall grid

Proposed implementation:

• This could be "toggle-able" with a short key to turn on/off this feature.
• Renders only in dev / staging
• This idea was inspired by Griddle
  • UVA uses Griddle and can be seen in a production site: (shift + control + L)
  • 
  • On the site of the creators' of Griddle:
  • 

🐛 Visual differences between Mona Sans locally and in CDN

Describe the bug

There are inconsistencies with the way Mona Sans is rendered in Primer Brand and in our CDN-hosted file.

Dotcom uses a CDN to deliver Mona Sans, and is likely using the most up-to-date version of the font files.

Reproduction steps

Replace local version of Primer Brand with CDN hosted version. E.g. https://github.githubassets.com/static/fonts/github/mona-sans.woff2

Expected behavior

Should render the same across all versions.

Screenshots

E.g.

alternating.between.mona-sans.in.CDN.mov

Browsers

No response

OS

No response

Add customisable heading level to FAQ.Question component

FAQ component headings use h4 elements, which aren't currently customisable.

The inability to change this becomes problematic when h4 is not appropriate during integration.

AXE will also report a violation of heading order, which is one of the consequences of not being able to customise this value.

{
    "id": "heading-order",
    "impact": "moderate",
    "tags": [
      "cat.semantics",
      "best-practice"
    ],
    "description": "Ensures the order of headings is semantically correct",
    "help": "Heading levels should only increase by one",
    "helpUrl": "https://dequeuniversity.com/rules/axe/4.6/heading-order?application=axeAPI",
    "nodes": [
      {
        "any": [
          {
            "id": "heading-order",
            "data": null,
            "relatedNodes": [],
            "impact": "moderate",
            "message": "Heading order invalid"
          }
        ],
        "all": [],
        "none": [],
        "impact": "moderate",
        "html": "<h4 class=\"Primer_Brand__Heading-module__Heading___IVpmp Primer_Brand__Heading-module__Heading--4___C9jDG Primer_Brand__Accordion-module__Accordion__summary-heading_____snF\">What is GitHub Galaxy?</h4>",
        "target": [
          "details:nth-child(1) > summary > .Primer_Brand__Accordion-module__Accordion__summary-heading_____snF.Primer_Brand__Heading-module__Heading--4___C9jDG"
        ],
        "failureSummary": "Fix any of the following:\n  Heading order invalid"
      }
    ]
  }

Solution:

This should be customisable using the as prop as we do in other components, with h4 becoming a default value to avoid risk of regressions.

[BUG - React] - SubdomainNavBar blur not filling entire width of header

Example:

Screen.Recording.2023-03-13.at.15.49.29.mov

Steps to reproduce:

1. Go to https://galaxy.github.com/
2. Resize viewport to make it smaller
3. Notice the blur is not filling the width

🐛 [BUG] - AnchorNav starting blank on mobile devices

Describe the bug

Under certain circumstances, the AnchorNav renders only the arrow and not the section title.

See the following example:

Screen.Recording.2023-06-23.at.12.58.11.mov

Reproduction steps

1. Go to https://primer.style/brand/storybook/?path=/story/components-anchornav-features--fewer-anchor-links&args=offset:500
2. Make sure the offset is, at least, `500px`
3. Use a narrow screen size
4. Scroll slowly until seeing the `AnchorNav` component
5. See error

Expected behavior

1. Go to https://primer.style/brand/storybook/?path=/story/components-anchornav-features--fewer-anchor-links&args=offset:500
2. Make sure the offset is, at least, `500px`
3. Use a narrow screen size
4. Scroll slowly until seeing the `AnchorNav` component
5. The `AnchorNav` shows the first section title

Screenshots

See video attached above.

Browsers

No response

OS

No response

Automate diffing and regression testing of design tokens in pull requests

React Brand components rely on design tokens to store and transfer most design decisions and logic to the UI.

Tokens are generated internally to this repo, and don't benefit from guardrails present in Primer Primitives.

Given the importance of the design tokens and the need for stability in our end-user output, we need to safeguard accidental changes to them.

One way to do this is to surface changes to tokens more promptly and easily..

Primer Primitives currently achieves this through CI automation scripts. Example below.

To take this idea further, we could also use visual regression testing alongside code diffing as accompanying documentation for reviewers, which is more visually engaging and easier to reason about. This can be done through Storybook fixtures.

Opening the floor to other ideas on how we can safeguard against accidental / incorrect changes to our tokens.

🐛 [BUG] - Animating hero (above the fold)

Describe the bug

This came up in https://github.com/github/githubuniverse.com/pull/1761#issuecomment-1649321470.

Reproduction steps

1. Click the Preview link in https://github.com/github/githubuniverse.com/pull/1761
2. You might have to reload and/or resize the browser window, but usually the "logo" does not animate in and stays blank
3. When you scroll up/down, animation works

Expected behavior

1. Click the Preview link in https://github.com/github/githubuniverse.com/pull/1761
2. The "logo" should animate

I played around with runOnce + visibilityOptions, but that doesn't seem to really help. Interestingly, if I remove all the other <AnimationProvider> on the page it works fine. 🤔

Maybe there is something that confuses the animationTrigger since the animation is in the hero (above the fold) and is not "scrolled into view". And option might be to add a animationTrigger='on-load' for animations that should just play as soon as the page loads (regardless of body scroll)?

Screenshots

See https://github.com/github/githubuniverse.com/pull/1761#issuecomment-1649321470

Browsers

Chrome

OS

Mac

[Figma] Provide horizontal, left-aligned heading layout option to FAQ

Add a FAQ component variant that supports a left-aligned horizontally stacked heading.

[Figma] Support arrow direction in Link component

Allow reversal of the arrow in Link for use-cases where we need to send users back to a previous page.

SubdomainNavBar not forwarding className correctly

SubdomainNavBar isn't forwarding the css class attribute value correctly, as it's overriding the internal classes due to spreading of props.

Relevant line:

brand/packages/react/src/SubdomainNavBar/SubdomainNavBar.tsx

Line 97 in fa0a2bf

<header className={styles['SubdomainNavBar']} {...rest}>

Should be fixed by destructuring the className from rest and composing into the existing className value.

Update specificity and className props

Context:

The compete page on resources hub utilized custom River components. Those components have been switched out for React Brand in this PR. I noted some pain points in the process and creating this issue as a result. The primary paint point involved needing to override this css targeting the image:

.River__visual img,
.River__visual video {
  width: 100%;
  height: 100%;
  object-fit: cover;
}

Issues:

Problem with CSS Specificity:

• When using multiple images, a user will want to override width: 100%, etc.
• The Primer Stylelint defines:

'selector-max-type': 0,
'selector-no-qualifying-type': true,

• But React Brand breaks that rule.
• This means the only way to override this rule is with !important

Potential Remedies

• Change the stylelint specification (recommended)
• Update the CSS to follow the stylelint

No className prop:

• I believe River.Content & River.Visual should accept a className prop. This can be addressed in a PR for #42 if you agree.
• In conjunction with no className prop, only Text, Heading and Link are allowed in River.Content, so there isn't an option to include an internal wrapper to target (e.g. wrapper Header and Text in div).

Thoughts on align prop

• What it does: Alignment of text content relative to the Visual position
• How I would expect alignment to be handled: Order of props. e.g.
  • Instead of align="left"

  <River.Content>...</River.Content>
  <River.Visual>...</River.Visual>
  

  • Instead of align="right"

  <River.Visual>...</River.Visual>
  <River.Content>...</River.Content>
  

• This feels as though this is more in line with the "composability"
• Could include a boolean center prop.
• These are just thoughts and not strongly held beliefs haha

🐛 [BUG - React] - FormControl.Required not indicated with Checkbox

Describe the bug

The following snippet (see "required" attribute):

<FormControl required>
  <FormControl.Label>Checkbox 1</FormControl.Label>
  <Checkbox name="day" value="" />
</FormControl>
<FormControl>
  <FormControl.Label>Checkbox 2</FormControl.Label>
  <Checkbox name="night" value="" />
</FormControl>

Doesn't indicate if the checkbox is required:

Reproduction steps

^ Above ^

Expected behavior

* is added to the checkbox title when it's required. Similar to the other fields here: https://primer.style/brand/storybook/?path=/story/components-forms-examples--git-hub-enterprise

Screenshots

^ Above ^

Browsers

Chrome

OS

Mac

🐛 [BUG] - MinimalFooter alignment for Links and Buttons

Describe the bug

When using MinimalFooter.Link as a and button together, the links aren't aligned correctly. In the example below the last link is rendered as button.

Reproduction steps

Use `MinimalFooter.Link` as `a` and `button` together.

Expected behavior

All links in the footer should be aligned properly.

Screenshots

Browsers

No response

OS

No response

Inconsistent outer spacing between components

Hey folks 👋,

I see there are some inconsistencies in components about horizontal spacing. That's especially noticeable when looking at components in narrow screen sizes. For example, the SectionIntro component uses a padding-block property that makes it have some horizontal space in narrow screens, while the River or the FAQ, for example, will be touching the screen's borders.

Right now, I'm applying some custom padding for both River and FAQ to create a consistent experience, but I think it would be great if we could make all components behave the same.

🐛 [BUG] - Arrow on CTAs not animating on hover

Describe the bug

Expected: CTA arrow should animate on hover
Current: No animation on hover

Example page: https://resources.github.com/devops/tools/compare/

the unhovered arrow starts with the classes Primer_Brand__ExpandableArrow-module__ExpandableArrow___rkfek, and on hover has the classes Primer_Brand__ExpandableArrow-module__ExpandableArrow___rkfek Primer_Brand__ExpandableArrow-module__ExpandableArrow--expanded___gajDr, but looks like there's no corresponding ExpandableArrow--expanded class in the css bundle.

Reproduction steps

go to https://resources.github.com/devops/tools/compare, hover over the start a free trial CTA.

Expected behavior

Expected: CTA arrow should animate on hover

Screenshots

https://user-images.githubusercontent.com/20496719/195380869-d07bed09-5ba1-4429-93ee-4bcea0a959f1.mp4

Browsers

Chrome

OS

Mac

[Accessibility] After resizing page to 320*256 focus moving to the hidden elements while navigating with the tab key.

As described in https://github.com/github/accessibility-audits/issues/5262

Users with low vision who find it difficult to read the text in small font uses enlarged text with re-size, will be impacted as at 400% zoom and users will be confused if the focus move to hidden element while using keyboard tab key.

Make SubdomainNavBar children optional

Some of the pages we build on resources.github.com are meant for campaigns only. Ideally, these pages minimize links to focus visitors on single CTAs. Could we make children for https://primer.style/brand/components/SubdomainNavBar optional, so that we can render it with just the GitHub logo and subdomain name?

[Figma] Support selectionVariants in ActionMenu

The ActionMenu component in Figma currently only supports single selections. We're adding support for a selectionVaraint property to match the React implementation and allow none variants (i.e., actions).