Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Link component to use CSS modules #4828

Merged
merged 12 commits into from
Aug 14, 2024
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
Loading