Skip to content

Commit

Permalink
Refactor Link component to use CSS modules (#4828)
Browse files Browse the repository at this point in the history
* Refactor Link to use CSS modules

* Remove needless disable

* Add newline

* Create quick-adults-buy.md

* Run tests inside fhte feature flag also

* Fix naming

* Rails interpolation got me

* test(e2e): update e2e tests for link (#4825)

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

* Remove example-nextjs

---------

Co-authored-by: Josh Black <joshblack@github.com>
Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
  • Loading branch information
3 people authored Aug 14, 2024
1 parent f3b08df commit 1a674f7
Show file tree
Hide file tree
Showing 18 changed files with 148 additions and 226 deletions.
3 changes: 1 addition & 2 deletions .changeset/pre.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
"codesandbox": "0.0.0",
"example-app-router": "0.0.0",
"example-consumer-test": "0.0.0",
"example-nextjs": "0.0.0",
"@primer/react": "36.27.0",
"rollup-plugin-import-css": "0.0.0",
"postcss-preset-primer": "0.0.0"
Expand All @@ -28,4 +27,4 @@
"twelve-tables-leave",
"young-meals-worry"
]
}
}
5 changes: 5 additions & 0 deletions .changeset/quick-adults-buy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Refactor Link component to use CSS modules using the feature flag `primer_react_css_modules`
1 change: 0 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ module.exports = {
'types/**/*',
'consumer-test/**/*',
'contributor-docs/adrs/*',
'examples/nextjs/**',
'examples/codesandbox/**',
// Note: this file is inlined from an external dependency
'packages/react/src/utils/polymorphic.ts',
Expand Down
46 changes: 24 additions & 22 deletions contributor-docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,25 +1,29 @@
# Contribution guidelines

1. [Roadmap](#roadmap)
2. [Before Getting Started](#before-getting-started)
3. [Discussing non-public features or products](#discussing-non-public-features-or-products)
4. [Developing Components](#developing-components)
- [Tools we use](#tools-we-use)
- [File Structure](#file-structure)
- [Component patterns](#component-patterns)
- [SSR compatibility](#ssr-compatibility)
- [Adding the sx prop](#adding-the-sx-prop)
- [Linting](#linting)
- [TypeScript support](#typescript-support)
- [Additional resources](#additional-resources)
5. [Writing documentation](#writing-documentation)
6. [Creating a pull request](#creating-a-pull-request)
- [Adding changeset to your pull request](#adding-changeset-to-your-pull-request)
- [What to expect after opening a pull request](#what-to-expect-after-opening-a-pull-request)
- [What we look for in reviews](#what-we-look-for-in-reviews)
- [Previewing your changes](#previewing-your-changes)
7. [Deploying](#deploying)
8. [Troubleshooting](#troubleshooting)
- [Contribution guidelines](#contribution-guidelines)
- [Roadmap](#roadmap)
- [Before Getting Started](#before-getting-started)
- [Proposing new components](#proposing-new-components)
- [Discussing non-public features or products](#discussing-non-public-features-or-products)
- [Developing components](#developing-components)
- [Tools we use](#tools-we-use)
- [File structure](#file-structure)
- [Component patterns](#component-patterns)
- [SSR compatibility](#ssr-compatibility)
- [Adding the `sx` prop](#adding-the-sx-prop)
- [Linting](#linting)
- [ESLint](#eslint)
- [Markdownlint](#markdownlint)
- [TypeScript support](#typescript-support)
- [Additional resources](#additional-resources)
- [Writing documentation](#writing-documentation)
- [Creating a pull request](#creating-a-pull-request)
- [Adding changeset to your pull request](#adding-changeset-to-your-pull-request)
- [What to expect after opening a pull request](#what-to-expect-after-opening-a-pull-request)
- [What we look for in reviews](#what-we-look-for-in-reviews)
- [Previewing your changes](#previewing-your-changes)
- [Deploying](#deploying)
- [Troubleshooting](#troubleshooting)

## Roadmap

Expand Down Expand Up @@ -148,8 +152,6 @@ We consider a component SSR-compatible if it...

We use [`eslint-plugin-ssr-friendly`](https://github.com/kopiro/eslint-plugin-ssr-friendly) to prevent misuse of DOM globals. If you see an error from this plugin, please fix it before merging your PR.

If your component doesn't use DOM globals, it likely won't cause layout shift during hydration. However, if you suspect that your component might cause layout shift, you can use the example Next.js app (`examples/nextjs`) to debug. Import and render your component in `examples/nextjs/src/pages/index.js` then run the example app with `cd examples/nextjs && npm run develop`.

### Adding the `sx` prop

Each component should accept a prop called `sx` that allows for setting theme-aware ad-hoc styles. See the [overriding styles](https://primer.style/react/overriding-styles) doc for more information on using the prop.
Expand Down
36 changes: 36 additions & 0 deletions e2e/components/Link.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ test.describe('Link', () => {
id: story.id,
globals: {
colorScheme: theme,
featureFlags: {
primer_react_css_modules: true,
},
},
})

Expand All @@ -47,6 +50,39 @@ test.describe('Link', () => {
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: story.id,
globals: {
colorScheme: theme,
featureFlags: {
primer_react_css_modules: true,
},
},
})
await expect(page).toHaveNoViolations()
})

test('default (styled-component) @vrt', async ({page}) => {
await visit(page, {
id: story.id,
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`Link.${story.title}.${theme}.png`)

// Hover state
await page.getByRole('link').hover()
expect(await page.screenshot()).toMatchSnapshot(`Link.${story.title}.${theme}.hover.png`)

// Focus state
await page.keyboard.press('Tab')
expect(await page.screenshot()).toMatchSnapshot(`Link.${story.title}.${theme}.focus.png`)
})

test('axe (styled-component) @aat', async ({page}) => {
await visit(page, {
id: story.id,
globals: {
Expand Down
1 change: 0 additions & 1 deletion examples/nextjs/.gitignore

This file was deleted.

5 changes: 0 additions & 5 deletions examples/nextjs/next-env.d.ts

This file was deleted.

5 changes: 0 additions & 5 deletions examples/nextjs/next.config.js

This file was deleted.

19 changes: 0 additions & 19 deletions examples/nextjs/package.json

This file was deleted.

19 changes: 0 additions & 19 deletions examples/nextjs/src/components/Layout.js

This file was deleted.

69 changes: 0 additions & 69 deletions examples/nextjs/src/components/Navigation.js

This file was deleted.

15 changes: 0 additions & 15 deletions examples/nextjs/src/pages/_app.js

This file was deleted.

6 changes: 0 additions & 6 deletions examples/nextjs/src/pages/index.js

This file was deleted.

10 changes: 0 additions & 10 deletions examples/nextjs/src/pages/tabs/[tab].js

This file was deleted.

30 changes: 0 additions & 30 deletions examples/nextjs/tsconfig.json

This file was deleted.

26 changes: 5 additions & 21 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 1a674f7

Please sign in to comment.