github / eslint-plugin-github

An opinionated collection of ESLint rules used by GitHub.
MIT License
295 stars 54 forks source link

Create rule: a11y-no-title-attribute #453

Closed kendallgassner closed 1 year ago

kendallgassner commented 1 year ago

What

A new rule to guard against the title attribute.

The title attribute is an inaccessible version of a tooltip. It is often used to provide context when text is truncated or add additional context to an interactive element like a link. This practice is exclusionary and prevents keyboard and mobile users from accessing information.

khiga8 commented 1 year ago

I wonder if title is ever used in Primer React as a prop that means something else 🤔, so we don't accidentally falsely flag it.

Do you know @tylerjdev?

TylerJDev commented 1 year ago

@khiga8 SelectPanel uses title for the heading, as well as ActionList for one of the subcomponents. Not sure if there are more instances, but I can gather any additional components that use title as a prop if it's helpful!

khiga8 commented 1 year ago

Thanks @tylerjdev! I wonder what the best approach would be to minimize false positives?

We could start conservatively and only run this check on HTML elements, if there's a way to identify that easily (all lower case?).

Alternatively, we could create an ignore-list that can be configured.

What do you think @tylerjdev and @kendallgassner?

kendallgassner commented 1 year ago

@khiga I lean towards a list of semantic html elements -- its less work to manage.

TylerJDev commented 1 year ago

I think I prefer the first option of only running this on native HTML elements. I thinking for an ignore list, that should primarily be Primer's responsibility, and might be hard to maintain if we hade on in this repo.

I do wonder if it'd make sense to make a duplicate rule in eslint-plugin-primer-react that would handle the same logic as this rule, but only for Primer components? There's a decent bit of title usage on components that do not support this as a custom prop, so it could be worth it, but would love to know any additional thoughts.

khiga8 commented 1 year ago

@tylerjdev That sounds good. I think starting with HTML only will minimize false positives so I like that too.

I do wonder if it'd make sense to make a duplicate rule in eslint-plugin-primer-react that would handle the same logic as this rule, but only for Primer components?

If we do introduce an ignore list config for this rule, it could be maintained by eslint-plugin-primer-react in recommended.js, so we wouldn't need to duplicate the rule. eslint-plugin-primer-react already maintains configs for this library (though it's out of date right now).

Note: the config in eslint-plugin-primer-react is out of date and should to be updated as part of https://github.com/primer/eslint-plugin-primer-react/issues/56.