From a8b3ed16e59c40c45ff4843df491838afb6931a1 Mon Sep 17 00:00:00 2001 From: Pavithra Kodmad Date: Fri, 8 Apr 2022 18:15:03 +1000 Subject: [PATCH 1/2] Add button focus styles (#2007) * Add button focus styles * Fix primary button focus styles * Fix safari fallback * Fix toggle switch snapshot * Create strange-jobs-add.md --- .changeset/strange-jobs-add.md | 5 + src/Button/styles.ts | 36 ++++-- .../__snapshots__/ActionMenu.test.tsx.snap | 18 ++- .../__snapshots__/Button.test.tsx.snap | 112 +++++++++++++----- .../__snapshots__/TextInput.test.tsx.snap | 54 ++++++--- 5 files changed, 162 insertions(+), 63 deletions(-) create mode 100644 .changeset/strange-jobs-add.md diff --git a/.changeset/strange-jobs-add.md b/.changeset/strange-jobs-add.md new file mode 100644 index 00000000000..4dba14fd7c0 --- /dev/null +++ b/.changeset/strange-jobs-add.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +Add button focus styles diff --git a/src/Button/styles.ts b/src/Button/styles.ts index a38b80b0b61..31f1ded7954 100644 --- a/src/Button/styles.ts +++ b/src/Button/styles.ts @@ -2,6 +2,17 @@ import {VariantType} from './types' import {Theme} from '../ThemeProvider' export const TEXT_ROW_HEIGHT = '20px' // custom value off the scale +const focusOutlineStyles = { + outline: '2px solid', + outlineColor: 'accent.fg', + outlineOffset: '-2px' +} +const fallbackFocus = { + ...focusOutlineStyles, + ':not(:focus-visible)': { + outline: 'solid 1px transparent' + } +} export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme) => { const style = { @@ -14,8 +25,9 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme }, // focus must come before :active so that the active box shadow overrides '&:focus:not([disabled])': { - boxShadow: `${theme?.shadows.btn.focusShadow}` + ...fallbackFocus }, + '&:focus-visible:not([disabled])': focusOutlineStyles, '&:active:not([disabled])': { backgroundColor: 'btn.activeBg', borderColor: 'btn.activeBorder' @@ -42,7 +54,12 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme }, // focus must come before :active so that the active box shadow overrides '&:focus:not([disabled])': { - boxShadow: `${theme?.shadows.btn.primary.focusShadow}` + boxShadow: 'inset 0 0 0 3px', + ...fallbackFocus + }, + '&:focus-visible:not([disabled])': { + ...focusOutlineStyles, + boxShadow: 'inset 0 0 0 3px' }, '&:active:not([disabled])': { backgroundColor: 'btn.primary.selectedBg', @@ -80,9 +97,9 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme }, // focus must come before :active so that the active box shadow overrides '&:focus:not([disabled])': { - borderColor: 'btn.danger.focusBorder', - boxShadow: `${theme?.shadows.btn.danger.focusShadow}` + ...fallbackFocus }, + '&:focus-visible:not([disabled])': focusOutlineStyles, '&:active:not([disabled])': { color: 'btn.danger.selectedText', backgroundColor: 'btn.danger.selectedBg', @@ -119,8 +136,9 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme }, // focus must come before :active so that the active box shadow overrides '&:focus:not([disabled])': { - boxShadow: `${theme?.shadows.btn.focusShadow}` + ...fallbackFocus }, + '&:focus-visible:not([disabled])': focusOutlineStyles, '&:active:not([disabled])': { backgroundColor: 'btn.selectedBg' }, @@ -152,10 +170,9 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme }, // focus must come before :active so that the active box shadow overrides '&:focus:not([disabled])': { - borderColor: 'btn.outline.focusBorder', - boxShadow: `${theme?.shadows.btn.outline.focusShadow}` + ...fallbackFocus }, - + '&:focus-visible:not([disabled])': focusOutlineStyles, '&:active:not([disabled])': { color: 'btn.outline.selectedText', backgroundColor: 'btn.outline.selectedBg', @@ -242,9 +259,6 @@ export const getBaseStyles = (theme?: Theme) => ({ userSelect: 'none', textDecoration: 'none', textAlign: 'center', - '&:focus': { - outline: 'none' - }, '&:disabled': { cursor: 'default' }, diff --git a/src/__tests__/__snapshots__/ActionMenu.test.tsx.snap b/src/__tests__/__snapshots__/ActionMenu.test.tsx.snap index 6a1ffc5e47e..a73796df9c5 100644 --- a/src/__tests__/__snapshots__/ActionMenu.test.tsx.snap +++ b/src/__tests__/__snapshots__/ActionMenu.test.tsx.snap @@ -44,10 +44,6 @@ exports[`ActionMenu renders consistently 1`] = ` box-shadow: 0 1px 0 rgba(27,31,36,0.04),inset 0 1px 0 rgba(255,255,255,0.25); } -.c1:focus { - outline: none; -} - .c1:disabled { cursor: default; color: #8c959f; @@ -86,7 +82,19 @@ exports[`ActionMenu renders consistently 1`] = ` } .c1:focus:not([disabled]) { - box-shadow: 0 0 0 3px rgba(9,105,218,0.3); + outline: 2px solid; + outline-color: #0969da; + outline-offset: -2px; +} + +.c1:focus:not([disabled]):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c1:focus-visible:not([disabled]) { + outline: 2px solid; + outline-color: #0969da; + outline-offset: -2px; } .c1:active:not([disabled]) { diff --git a/src/__tests__/__snapshots__/Button.test.tsx.snap b/src/__tests__/__snapshots__/Button.test.tsx.snap index 48a7b73120c..132275194b1 100644 --- a/src/__tests__/__snapshots__/Button.test.tsx.snap +++ b/src/__tests__/__snapshots__/Button.test.tsx.snap @@ -33,10 +33,6 @@ exports[`Button renders consistently 1`] = ` box-shadow: 0 1px 0 rgba(27,31,36,0.04),inset 0 1px 0 rgba(255,255,255,0.25); } -.c0:focus { - outline: none; -} - .c0:disabled { cursor: default; color: #8c959f; @@ -75,7 +71,19 @@ exports[`Button renders consistently 1`] = ` } .c0:focus:not([disabled]) { - box-shadow: 0 0 0 3px rgba(9,105,218,0.3); + outline: 2px solid; + outline-color: #0969da; + outline-offset: -2px; +} + +.c0:focus:not([disabled]):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c0:focus-visible:not([disabled]) { + outline: 2px solid; + outline-color: #0969da; + outline-offset: -2px; } .c0:active:not([disabled]) { @@ -131,10 +139,6 @@ exports[`Button styles danger button appropriately 1`] = ` box-shadow: undefined; } -.c0:focus { - outline: none; -} - .c0:disabled { cursor: default; color: btn.danger.disabledText; @@ -186,8 +190,19 @@ exports[`Button styles danger button appropriately 1`] = ` } .c0:focus:not([disabled]) { - border-color: btn.danger.focusBorder; - box-shadow: undefined; + outline: 2px solid; + outline-color: accent.fg; + outline-offset: -2px; +} + +.c0:focus:not([disabled]):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c0:focus-visible:not([disabled]) { + outline: 2px solid; + outline-color: accent.fg; + outline-offset: -2px; } .c0:active:not([disabled]) { @@ -255,10 +270,6 @@ exports[`Button styles icon only button to make it a square 1`] = ` box-shadow: undefined,undefined; } -.c0:focus { - outline: none; -} - .c0:disabled { cursor: default; color: primer.fg.disabled; @@ -281,7 +292,19 @@ exports[`Button styles icon only button to make it a square 1`] = ` } .c0:focus:not([disabled]) { - box-shadow: undefined; + outline: 2px solid; + outline-color: accent.fg; + outline-offset: -2px; +} + +.c0:focus:not([disabled]):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c0:focus-visible:not([disabled]) { + outline: 2px solid; + outline-color: accent.fg; + outline-offset: -2px; } .c0:active:not([disabled]) { @@ -358,10 +381,6 @@ exports[`Button styles invisible button appropriately 1`] = ` box-shadow: none; } -.c0:focus { - outline: none; -} - .c0:disabled { cursor: default; color: primer.fg.disabled; @@ -400,7 +419,19 @@ exports[`Button styles invisible button appropriately 1`] = ` } .c0:focus:not([disabled]) { - box-shadow: undefined; + outline: 2px solid; + outline-color: accent.fg; + outline-offset: -2px; +} + +.c0:focus:not([disabled]):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c0:focus-visible:not([disabled]) { + outline: 2px solid; + outline-color: accent.fg; + outline-offset: -2px; } .c0:active:not([disabled]) { @@ -461,10 +492,6 @@ exports[`Button styles outline button appropriately 1`] = ` background-color: btn.bg; } -.c0:focus { - outline: none; -} - .c0:disabled { cursor: default; color: btn.outline.disabledText; @@ -516,8 +543,19 @@ exports[`Button styles outline button appropriately 1`] = ` } .c0:focus:not([disabled]) { - border-color: btn.outline.focusBorder; - box-shadow: undefined; + outline: 2px solid; + outline-color: accent.fg; + outline-offset: -2px; +} + +.c0:focus:not([disabled]):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c0:focus-visible:not([disabled]) { + outline: 2px solid; + outline-color: accent.fg; + outline-offset: -2px; } .c0:active:not([disabled]) { @@ -584,10 +622,6 @@ exports[`Button styles primary button appropriately 1`] = ` box-shadow: undefined; } -.c0:focus { - outline: none; -} - .c0:disabled { cursor: default; color: btn.primary.disabledText; @@ -630,7 +664,21 @@ exports[`Button styles primary button appropriately 1`] = ` } .c0:focus:not([disabled]) { - box-shadow: undefined; + box-shadow: inset 0 0 0 3px; + outline: 2px solid; + outline-color: accent.fg; + outline-offset: -2px; +} + +.c0:focus:not([disabled]):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c0:focus-visible:not([disabled]) { + outline: 2px solid; + outline-color: accent.fg; + outline-offset: -2px; + box-shadow: inset 0 0 0 3px; } .c0:active:not([disabled]) { diff --git a/src/__tests__/__snapshots__/TextInput.test.tsx.snap b/src/__tests__/__snapshots__/TextInput.test.tsx.snap index 6c82e7e7ba0..f8408bbeaac 100644 --- a/src/__tests__/__snapshots__/TextInput.test.tsx.snap +++ b/src/__tests__/__snapshots__/TextInput.test.tsx.snap @@ -1189,10 +1189,6 @@ exports[`TextInput renders trailingAction icon button 1`] = ` box-shadow: none; } -.c3:focus { - outline: none; -} - .c3:disabled { cursor: default; color: #8c959f; @@ -1215,7 +1211,19 @@ exports[`TextInput renders trailingAction icon button 1`] = ` } .c3:focus:not([disabled]) { - box-shadow: 0 0 0 3px rgba(9,105,218,0.3); + outline: 2px solid; + outline-color: #0969da; + outline-offset: -2px; +} + +.c3:focus:not([disabled]):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c3:focus-visible:not([disabled]) { + outline: 2px solid; + outline-color: #0969da; + outline-offset: -2px; } .c3:active:not([disabled]) { @@ -1632,10 +1640,6 @@ exports[`TextInput renders trailingAction text button 1`] = ` box-shadow: none; } -.c2:focus { - outline: none; -} - .c2:disabled { cursor: default; color: #8c959f; @@ -1674,7 +1678,19 @@ exports[`TextInput renders trailingAction text button 1`] = ` } .c2:focus:not([disabled]) { - box-shadow: 0 0 0 3px rgba(9,105,218,0.3); + outline: 2px solid; + outline-color: #0969da; + outline-offset: -2px; +} + +.c2:focus:not([disabled]):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c2:focus-visible:not([disabled]) { + outline: 2px solid; + outline-color: #0969da; + outline-offset: -2px; } .c2:active:not([disabled]) { @@ -1845,10 +1861,6 @@ exports[`TextInput renders trailingAction text button with a tooltip 1`] = ` box-shadow: none; } -.c3:focus { - outline: none; -} - .c3:disabled { cursor: default; color: #8c959f; @@ -1887,7 +1899,19 @@ exports[`TextInput renders trailingAction text button with a tooltip 1`] = ` } .c3:focus:not([disabled]) { - box-shadow: 0 0 0 3px rgba(9,105,218,0.3); + outline: 2px solid; + outline-color: #0969da; + outline-offset: -2px; +} + +.c3:focus:not([disabled]):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c3:focus-visible:not([disabled]) { + outline: 2px solid; + outline-color: #0969da; + outline-offset: -2px; } .c3:active:not([disabled]) { From 6b387465274519617504e01a7ddad47df4a03ccb Mon Sep 17 00:00:00 2001 From: Rez Date: Fri, 8 Apr 2022 19:09:02 +0100 Subject: [PATCH 2/2] Add pre-release checklist (#2015) --- .github/release_template.md | 28 ++++++++++++++++++++ .github/workflows/pull_request.yml | 41 ++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 .github/release_template.md create mode 100644 .github/workflows/pull_request.yml diff --git a/.github/release_template.md b/.github/release_template.md new file mode 100644 index 00000000000..0c0ebf20b44 --- /dev/null +++ b/.github/release_template.md @@ -0,0 +1,28 @@ +## ❗ Pre-merge checklist + +Please ensure these items are checked before merging. + +### 🔎 Smoke test + +- [ ] All CI checks pass +- [ ] Docs and Storybook open in a browser +- [ ] Successful integration test with GitHub Projects as a primary consumer of Primer React + - [ ] Install the Release Candidate + - [ ] Verify no new build errors appear + - [ ] Verify no new linting errors appear + - [ ] Verify no new browser console errors appear + - [ ] Verify unit tests and E2E tests pass + - [ ] Manually test critical paths (Tip: Use the provided tests project boards) + - [ ] Manually test release-specific bugfixes and/or features work as described +- [ ] Works in CodeSandbox or StackBlitz + - [ ] New components render successfully + - [ ] (optional) Tested in both SPA and SSR apps if release contains build changes + +### 🤔 Sanity test + +- [ ] All bugfixes in this release have resolved their corrosponding issues +- [ ] All new features in this release have been tested and verified as compatible with GitHub Projects +- [ ] No noticeable regressions have not been introduced as a result of changes in this release +- [ ] Release notes accurately describe the changes made + +Please also leave any testing notes as a comment on this pull request. In particular, describing any issues encountered during your testing. This is helpful in providing historical context to maintainers. diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml new file mode 100644 index 00000000000..62dc5a1aaed --- /dev/null +++ b/.github/workflows/pull_request.yml @@ -0,0 +1,41 @@ +name: Pull request + +on: + pull_request: + +jobs: + release-template: + # Only run on Release Tracking branch + if: ${{ github.head_ref == 'changeset-release/main' }} + + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v2 + + - name: Set up Node.js + uses: actions/setup-node@v3 + with: + node-version: 16 + + - name: Get or Create Comment + uses: actions/github-script@v5 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const fs = require('fs') + const body = await fs.readFileSync('.github/release_template.md', 'utf8') + const result = await github.rest.issues.listComments({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo + }); + const botComments = result.data.filter(c => c.user.login == 'github-actions[bot]') + if (!botComments.length) { + await github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: body.replace('{{PR_AUTHOR}}', context.payload.sender.login) + }) + }