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

WIP: Check code examples with axe #3168

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

HalvorHaugan
Copy link
Contributor

@HalvorHaugan HalvorHaugan commented Sep 21, 2024

I thought we should check our components for a11y issues and invalid HTML. I think the easiest way is to check the examples? It might be good to check the examples anyways. Should probably also check the templates.

It seems that axe doesn't properly check for invalid HTML, so maybe we need a separate check for that?

Copy link

changeset-bot bot commented Sep 21, 2024

⚠️ No Changeset found

Latest commit: 5fc3b5a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Collaborator

@KenAJoh KenAJoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, seems like a great idea to implement 👏 As an alternative we could use https://storybook.js.org/docs/writing-tests/test-runner since we already have the storybook setup for it (if you were looking for alternatives). But since we have playwright setup already the current implementation sees like a much simpler solution 👍

id="ds-example"
className={variant === "static" ? styles.exampleStatic : undefined}
<>
<SEO title="Kodeeksempel" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SEO manglet kanskje her? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

La til den fordi axe klaget på at <title> manglet.

@@ -46,10 +46,11 @@ const config: PlaywrightTestConfig = {
...devices["Desktop Chrome"],
},
...(process.env.FULL_TEST
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to have the same test suite run for both cases, we could also just remove this FULL_TEST env var.

},
{
// TODO: Revert all changes to this file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guessing this is WIP stuff? (there's a lot of comments that we could probably strip as well before this goes live / un-draft the PR) :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this branch is WIP and not ready for review. But ideas/suggestions are welcome!

@JulianNymark
Copy link
Contributor

I'm all for this! 🙌 We tend to use AXE in the browser, but it can easily get forgotten by individual devs 😎.

@HalvorHaugan HalvorHaugan changed the title Check code examples with axe WIP: Check code examples with axe Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants