Skip to content

Feat/eslint a11y plugin #296

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

luukbrauckmann
Copy link
Member

@luukbrauckmann luukbrauckmann commented Jul 24, 2025

Changes

  • Add eslint a11y plugin for Astro components
  • Fix errors caused by this plugin

This plugin checks for accessibility problem and forces you to use necessary attributes.

Associated issue

N.A.

How to test

  1. Check commit history. There you can see that the linter is failing. If you open it you can see the errors.
  2. In the last commit(s) you can see the changes to fix it.

Checklist

  • I have performed a self-review of my own code
  • I have made sure that my PR is easy to review (not too big, includes comments)
  • I have made updated relevant documentation files (in project README, docs/, etc)
  • I have added a decision log entry if the change affects the architecture or changes a significant technology
  • I have notified a reviewer

The commit fixes several TypeScript-related issues, improves type
safety, and makes type usage more consistent across multiple components.
This includes proper HTMLAttributes usage, explicit HTMLTag typing, and
better Props type definitions.
recommended - Because this is not cjs the flat/ prefix is not needed.
Copy link

cloudflare-workers-and-pages bot commented Jul 24, 2025

Deploying head-start with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8942b4e
Status: ✅  Deploy successful!
Preview URL: https://95a11eb7.head-start.pages.dev
Branch Preview URL: https://feat-eslint-a11y-plugin.head-start.pages.dev

View logs


<style>
ul {
list-style: none;
list-style-type: "";
Copy link
Member Author

Choose a reason for hiding this comment

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

When using the list-style-type: ""; CSS property with an empty string value, you remove the visual list markers (bullets, numbers, etc.) while preserving the semantic meaning and accessibility properties of the list for screen readers and other assistive technologies. I've also updated the readme for the UnstyledList with this.

Copy link
Member

@bazottie bazottie Jul 25, 2025

Choose a reason for hiding this comment

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

I've seen or had this discussion before in head-start related projects. If I remember correctly, this solution does not work for all browsers, so resetting the styling with list-style: none; with role="list" has better cross-browser support. Moreover, in the described behaviour this component would be redundant. What is the reason behind refactoring this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't really find a way to make sure eslint knows it does contain a <track>. I don't know if it's because it's coming from another file or if it can be empty, so I disabled the rule for the video elements

@luukbrauckmann luukbrauckmann marked this pull request as ready for review July 24, 2025 12:59
@luukbrauckmann luukbrauckmann self-assigned this Jul 24, 2025
Base automatically changed from fix/eslint-ignore-pattern to main July 24, 2025 14:09
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.

2 participants