Merge branch 'main' into siddharth/actionlist-doc-comment

primer · Nov 15, 2021 · c7c47e1 · c7c47e1
2 parents 72e6643 + d2090b5
commit c7c47e1
Show file tree

Hide file tree

Showing 19 changed files with 221 additions and 77 deletions.
diff --git a/.changeset/actionlist-keyboard-a11y.md b/.changeset/actionlist-keyboard-a11y.md
@@ -0,0 +1,5 @@
+---
+'@primer/components': patch
+---
+
+ActionList: Improve keyboard accessibility with focus styles cross browser
diff --git a/.changeset/state-label-draft-issue.md b/.changeset/state-label-draft-issue.md
@@ -0,0 +1,5 @@
+---
+"@primer/components": patch
+---
+
+Add support for draft issues to the StateLabel component
diff --git a/docs/content/StateLabel.md b/docs/content/StateLabel.md
@@ -14,6 +14,7 @@ Use StateLabel components to show the status of an issue or pull request.
   <StateLabel status="pullClosed">Closed</StateLabel>
   <StateLabel status="pullMerged">Merged</StateLabel>
   <StateLabel status="draft">Draft</StateLabel>
+  <StateLabel status="issueDraft">Draft</StateLabel>
 </>
 ```
 
@@ -29,7 +30,7 @@ StateLabel components get `COMMON` system props. Read our [System Props](/system
 
 ## Component props
 
-| Name    | Type   | Default  | Description                                                                                       |
-| :------ | :----- | :------: | :------------------------------------------------------------------------------------------------ |
-| variant | String | 'normal' | a value of `small` or `normal` results in a smaller or larger version of the StateLabel.          |
-| status  | String |          | Can be one of `issueOpened`, `issueClosed`, `pullOpened`, `pullClosed`, `pullMerged`, or `draft`. |
+| Name    | Type   | Default  | Description                                                                                                    |
+| :------ | :----- | :------: | :------------------------------------------------------------------------------------------------------------- |
+| variant | String | 'normal' | a value of `small` or `normal` results in a smaller or larger version of the StateLabel.                       |
+| status  | String |          | Can be one of `issueOpened`, `issueClosed`, `pullOpened`, `pullClosed`, `pullMerged`, `draft` or `issueDraft`. |
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -43,7 +43,7 @@
   "author": "GitHub, Inc.",
   "license": "MIT",
   "dependencies": {
-    "@primer/octicons-react": "^13.0.0",
+    "@primer/octicons-react": "^16.1.0",
     "@primer/primitives": "6.1.0",
     "@radix-ui/react-polymorphic": "0.0.14",
     "@react-aria/ssr": "3.1.0",

diff --git a/src/ActionList2/Item.tsx b/src/ActionList2/Item.tsx
@@ -94,7 +94,7 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
       onSelect = () => null,
       sx: sxProp = {},
       id,
-      _PrivateItemWrapper = ({children}) => <>{children}</>,
+      _PrivateItemWrapper,
       ...props
     },
     forwardedRef
@@ -109,9 +109,9 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
       fontSize: 1,
       paddingY: '6px', // custom value off the scale
       lineHeight: TEXT_ROW_HEIGHT,
-      marginX: listVariant === 'inset' ? 2 : 0,
       minHeight: 5,
-      borderRadius: 2,
+      marginX: listVariant === 'inset' ? 2 : 0,
+      borderRadius: listVariant === 'inset' ? 2 : 0,
       transition: 'background 33.333ms linear',
       color: getVariantStyles(variant, disabled).color,
       textDecoration: 'none', // for as="a"
@@ -121,11 +121,17 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
           backgroundColor: `actionListItem.${variant}.hoverBg`,
           color: getVariantStyles(variant, disabled).hoverColor
         },
-        ':focus:not([aria-disabled])': {
+        ':focus:not([data-focus-visible-added])': {
           backgroundColor: `actionListItem.${variant}.selectedBg`,
           color: getVariantStyles(variant, disabled).hoverColor,
           outline: 'none'
         },
+        '&[data-focus-visible-added]': {
+          // we don't use :focus-visible because not all browsers (safari) have it yet
+          outline: 'none',
+          border: `2 solid`,
+          boxShadow: `0 0 0 2px ${theme?.colors.accent.emphasis}`
+        },
         ':active:not([aria-disabled])': {
           backgroundColor: `actionListItem.${variant}.activeBg`,
           color: getVariantStyles(variant, disabled).hoverColor
@@ -147,14 +153,16 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
         borderColor: 'var(--divider-color, transparent)'
       },
       // show between 2 items
-      ':not(:first-of-type):not([aria-selected=true])': {'--divider-color': theme?.colors.actionListItem.inlineDivider},
+      ':not(:first-of-type)': {'--divider-color': theme?.colors.actionListItem.inlineDivider},
       // hide divider after dividers & group header, with higher importance!
       '[data-component="ActionList.Divider"] + &': {'--divider-color': 'transparent !important'},
       // hide border on current and previous item
-      '&:hover:not([aria-disabled]), &:focus:not([aria-disabled])': {'--divider-color': 'transparent'},
-      '&:hover:not([aria-disabled]) + &, &:focus:not([aria-disabled]) + &': {'--divider-color': 'transparent'},
-      // hide border around selected item
-      '&[aria-selected=true] + &': {'--divider-color': 'transparent'}
+      '&:hover:not([aria-disabled]), &:focus:not([aria-disabled]), &[data-focus-visible-added]:not([aria-disabled])': {
+        '--divider-color': 'transparent'
+      },
+      '&:hover:not([aria-disabled]) + &, &:focus:not([aria-disabled]) + &, &[data-focus-visible-added] + li': {
+        '--divider-color': 'transparent'
+      }
     }
 
     const clickHandler = React.useCallback(
@@ -165,26 +173,40 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
       [onSelect, disabled]
     )
 
+    const keyPressHandler = React.useCallback(
+      event => {
+        if (disabled) return
+
+        if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) {
+          onSelect(event)
+        }
+      },
+      [onSelect, disabled]
+    )
+
     // use props.id if provided, otherwise generate one.
     const labelId = useSSRSafeId(id)
     const inlineDescriptionId = useSSRSafeId(id && `${id}--inline-description`)
     const blockDescriptionId = useSSRSafeId(id && `${id}--block-description`)
 
+    const ItemWrapper = _PrivateItemWrapper || React.Fragment
+
     return (
       <Slots context={{variant, disabled, inlineDescriptionId, blockDescriptionId}}>
         {slots => (
           <LiBox
             ref={forwardedRef}
             sx={merge(styles, sxProp as SxProp)}
             onClick={clickHandler}
+            onKeyPress={keyPressHandler}
             aria-selected={selected}
             aria-disabled={disabled ? true : undefined}
-            tabIndex={disabled ? undefined : -1}
+            tabIndex={disabled || _PrivateItemWrapper ? undefined : 0}
             aria-labelledby={`${labelId} ${slots.InlineDescription ? inlineDescriptionId : ''}`}
             aria-describedby={slots.BlockDescription ? blockDescriptionId : undefined}
             {...props}
           >
-            <_PrivateItemWrapper>
+            <ItemWrapper>
               <Selection selected={selected} disabled={disabled} />
               {slots.LeadingVisual}
               <Box
@@ -205,7 +227,7 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
                 </ConditionalBox>
                 {slots.BlockDescription}
               </Box>
-            </_PrivateItemWrapper>
+            </ItemWrapper>
           </LiBox>
         )}
       </Slots>

diff --git a/src/StateLabel.tsx b/src/StateLabel.tsx
@@ -1,4 +1,11 @@
-import {GitMergeIcon, GitPullRequestIcon, IssueClosedIcon, IssueOpenedIcon, QuestionIcon} from '@primer/octicons-react'
+import {
+  GitMergeIcon,
+  GitPullRequestIcon,
+  IssueClosedIcon,
+  IssueDraftIcon,
+  IssueOpenedIcon,
+  QuestionIcon
+} from '@primer/octicons-react'
 import React from 'react'
 import styled from 'styled-components'
 import {variant} from 'styled-system'
@@ -13,7 +20,8 @@ const octiconMap = {
   issueClosed: IssueClosedIcon,
   pullClosed: GitPullRequestIcon,
   pullMerged: GitMergeIcon,
-  draft: GitPullRequestIcon
+  draft: GitPullRequestIcon,
+  issueDraft: IssueDraftIcon
 }
 
 const colorVariants = variant({
@@ -42,6 +50,10 @@ const colorVariants = variant({
     draft: {
       backgroundColor: 'neutral.emphasis',
       color: 'fg.onEmphasis'
+    },
+    issueDraft: {
+      backgroundColor: 'neutral.emphasis',
+      color: 'fg.onEmphasis'
     }
   }
 })

diff --git a/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap b/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap
@@ -972,6 +972,7 @@ Array [
                 style={
                   Object {
                     "display": "inline-block",
+                    "overflow": "visible",
                     "userSelect": "none",
                     "verticalAlign": "text-bottom",
                   }

diff --git a/src/__tests__/__snapshots__/CircleBadge.test.tsx.snap b/src/__tests__/__snapshots__/CircleBadge.test.tsx.snap
@@ -21,6 +21,7 @@ exports[`CircleBadge CircleBadge.Icon renders consistently 1`] = `
   style={
     Object {
       "display": "inline-block",
+      "overflow": "visible",
       "userSelect": "none",
       "verticalAlign": "text-bottom",
     }

diff --git a/src/__tests__/__snapshots__/CircleOcticon.test.tsx.snap b/src/__tests__/__snapshots__/CircleOcticon.test.tsx.snap
@@ -52,6 +52,7 @@ exports[`CircleOcticon renders consistently 1`] = `
       style={
         Object {
           "display": "inline-block",
+          "overflow": "visible",
           "userSelect": "none",
           "verticalAlign": "text-bottom",
         }

diff --git a/src/__tests__/__snapshots__/Dialog.test.tsx.snap b/src/__tests__/__snapshots__/Dialog.test.tsx.snap
@@ -163,6 +163,7 @@ Array [
         style={
           Object {
             "display": "inline-block",
+            "overflow": "visible",
             "userSelect": "none",
             "verticalAlign": "text-bottom",
           }

diff --git a/src/__tests__/__snapshots__/DropdownMenu.test.tsx.snap b/src/__tests__/__snapshots__/DropdownMenu.test.tsx.snap
@@ -95,6 +95,7 @@ exports[`DropdownMenu renders consistently 1`] = `
     style={
       Object {
         "display": "inline-block",
+        "overflow": "visible",
         "userSelect": "none",
         "verticalAlign": "text-bottom",
       }

diff --git a/src/__tests__/__snapshots__/SelectMenu.test.tsx.snap b/src/__tests__/__snapshots__/SelectMenu.test.tsx.snap
@@ -352,6 +352,7 @@ exports[`SelectMenu right-aligned modal has right: 0px 1`] = `
             style={
               Object {
                 "display": "inline-block",
+                "overflow": "visible",
                 "userSelect": "none",
                 "verticalAlign": "text-bottom",
               }
@@ -383,6 +384,7 @@ exports[`SelectMenu right-aligned modal has right: 0px 1`] = `
             style={
               Object {
                 "display": "inline-block",
+                "overflow": "visible",
                 "userSelect": "none",
                 "verticalAlign": "text-bottom",
               }
@@ -418,6 +420,7 @@ exports[`SelectMenu right-aligned modal has right: 0px 1`] = `
             style={
               Object {
                 "display": "inline-block",
+                "overflow": "visible",
                 "userSelect": "none",
                 "verticalAlign": "text-bottom",
               }
@@ -448,6 +451,7 @@ exports[`SelectMenu right-aligned modal has right: 0px 1`] = `
             style={
               Object {
                 "display": "inline-block",
+                "overflow": "visible",
                 "userSelect": "none",
                 "verticalAlign": "text-bottom",
               }

diff --git a/src/__tests__/__snapshots__/SelectPanel.test.tsx.snap b/src/__tests__/__snapshots__/SelectPanel.test.tsx.snap
@@ -108,6 +108,7 @@ exports[`SelectPanel renders consistently 1`] = `
       style={
         Object {
           "display": "inline-block",
+          "overflow": "visible",
           "userSelect": "none",
           "verticalAlign": "text-bottom",
         }