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

Fix a few minor accessibility issues with examples #4205

Merged

Conversation

coliff
Copy link
Contributor

@coliff coliff commented May 30, 2024

  • Link labels to inputs
  • Add missing title and viewport to GL Stats page

Launch Checklist

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality.
  • Document any changes to public APIs.
  • Post benchmark scores.
  • Add an entry to CHANGELOG.md under the ## main section.

- Link labels to inputs
- Add missing title and viewport to GL Stats page
Copy link
Collaborator

@HarelM HarelM left a comment

Choose a reason for hiding this comment

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

Gl stats is internal for testing, but I don't mind having it improved.

@HarelM HarelM enabled auto-merge (squash) May 30, 2024 15:43
@codecov-commenter
Copy link

codecov-commenter commented May 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.69%. Comparing base (3164f1d) to head (f452979).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4205      +/-   ##
==========================================
+ Coverage   87.52%   87.69%   +0.17%     
==========================================
  Files         242      242              
  Lines       33080    33080              
  Branches     2178     2159      -19     
==========================================
+ Hits        28952    29009      +57     
+ Misses       3131     3085      -46     
+ Partials      997      986      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@coliff
Copy link
Contributor Author

coliff commented May 30, 2024

I was going to suggest adding a HTMLHint test to make sure there are no bugs with all the HTML pages in the repo. For now you can run:

npx htmlhint **/*.html

for a .htmlhintrc config in the root with the following config:

{
  "alt-require": true,
  "attr-lowercase": true,
  "attr-no-duplication": true,
  "attr-unsafe-chars": true,
  "attr-value-double-quotes": false,
  "attr-value-not-empty": false,
  "doctype-first": false,
  "doctype-html5": true,
  "head-script-disabled": false,
  "href-abs-or-rel": false,
  "html-lang-require": true,
  "id-class-ad-disabled": true,
  "id-class-value": false,
  "id-unique": true,
  "inline-script-disabled": false,
  "inline-style-disabled": false,
  "spec-char-escape": false,
  "src-not-empty": true,
  "style-disabled": false,
  "tag-pair": false,
  "tag-self-close": false,
  "tagname-lowercase": true,
  "title-require": true
}

@HarelM
Copy link
Collaborator

HarelM commented May 30, 2024

But we have eslint with the html plugin already running... Are there rules there we can add?

@HarelM HarelM merged commit aa435b6 into maplibre:main May 30, 2024
15 checks passed
@coliff coliff deleted the dev/coliff/minor-accessibility-fixes-examples branch May 31, 2024 05:14
@coliff
Copy link
Contributor Author

coliff commented May 31, 2024

But we have eslint with the html plugin already running... Are there rules there we can add?

I'll take a look!

@coliff
Copy link
Contributor Author

coliff commented May 31, 2024

@HarelM - re: eslint with the html plugin already running

https://www.npmjs.com/package/eslint-plugin-html

This plugin focuses on applying ESLint rules on inline scripts contained in HTML. It does not provide any rule related to HTML.

So maybe need to add this too: https://html-eslint.org/

@HarelM
Copy link
Collaborator

HarelM commented May 31, 2024

Feel free to add this. although, all these plugins are a hazard when eslint is releasing a breaking change version like they did with 9.0, as these tend to not be maintained, as start to fail.
But we can always remove them when they get stale I guess...

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