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

Add explicit-heading rule #66

Merged
merged 7 commits into from
Jul 13, 2023
Merged

Add explicit-heading rule #66

merged 7 commits into from
Jul 13, 2023

Conversation

TylerJDev
Copy link
Contributor

Adds explicit-heading rule. This rule disallows the following:

  • Using Heading without providing a heading level via as, (e.g. <Heading>)
  • Using as for any element that is not a heading (h1-h6)

Additional context: #57.

@changeset-bot
Copy link

changeset-bot bot commented Jul 10, 2023

🦋 Changeset detected

Latest commit: 4560876

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-plugin-primer-react Major

Not sure what this means? Click here to learn what changesets are.

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

@TylerJDev TylerJDev marked this pull request as ready for review July 11, 2023 19:35
@khiga8
Copy link
Contributor

khiga8 commented Jul 11, 2023

I can review more carefully tomorrow, but do you think this is considered an accessibility rule, and would we want to include this rule in the accessibility scorecard?

If so, we should prefix the name with a11y- which would allow us to flag it as an accessibility rule.

Copy link
Contributor

@khiga8 khiga8 left a comment

Choose a reason for hiding this comment

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

I have one open question about prefixing this lint rule with a11y- so we can identify it as an accessibility-related rule more easily and eventualy track it in our scorecard. (We're still figuring out the rollout system around that in https://github.com/github/accessibility/issues/3868).

Aside from that, I have no concerns. This is awesome! 🎉

@TylerJDev
Copy link
Contributor Author

@khiga8 Yup I can definitely add that!

@TylerJDev TylerJDev requested review from a team and broccolinisoup July 12, 2023 21:09
Copy link
Contributor

@khiga8 khiga8 left a comment

Choose a reason for hiding this comment

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

Great work 🎉

@TylerJDev TylerJDev merged commit dbb58f2 into main Jul 13, 2023
@TylerJDev TylerJDev deleted the tylerjdev/explicit-heading branch July 13, 2023 14:51
@primer-css primer-css mentioned this pull request Jul 13, 2023
Copy link
Member

@broccolinisoup broccolinisoup left a comment

Choose a reason for hiding this comment

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

Sorry I missed this! Thanks for a new shiny rule 🤩 Only thing I would say is that we should add this rule to the README.md.

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