-
Notifications
You must be signed in to change notification settings - Fork 10
Closed
Description
I would like to suggest that we create a lint rule that prevents using only onClick
with ActionList.Item
and that would auto-fix to use onSelect
instead.
Context
I've recently fixed a number of accessibility bugs where behavior was only accessible with the mouse because we passed onClick
to an action list item, instead of using the onSelect
attribute.
Examples:
- https://github.com/github/github/pull/333584/files#diff-e126663a316e199308eb3b48cc3837f0bad69f108e36538afc629f92051ac85bR210
- https://github.com/github/github/pull/333599/files#diff-17bdd00340af50ddd6302d900b33310f25328339c12cfd22b9a68792e50fa76dR198
- https://github.com/github/github/pull/333618/files#diff-75f8395823a4cf1bc91eee589beaa11d22c36c9ebf90ff5a352102f0a348b71eR105
While onClick
is not completely invalid to put on ActionList.Item
, if it is the only means by which an item can be activated, then it creates an inaccessible experience.
Using code search, I can also see some other code examples which might be problematic:
- https://github.com/search?q=repo%3Agithub%2Fgithub+%2FActionList.Item%5Cn.*onClick%2F&type=code
- https://github.com/search?q=repo%3Agithub%2Fgithub+%2FActionList.Item.*onClick%3D%2F&type=code
- https://github.com/search?q=repo%3Agithub%2Fgithub+%2FActionList.Item%5Cn.*%5Cn.*onClick%3D%2F&type=code
- https://github.com/search?q=repo%3Agithub%2Fgithub+%2FActionList.Item%5Cn.*%5Cn.*%5Cn.*onClick%3D%2F&type=code
Examples
Code that should be considered invalid by this rule:
<ActionList.Item onClick={...}>
Code that should be considered valid by this rule:
<ActionList.Item onSelect={...}>
<ActionList.Item onSelect={...} onClick={...}>
siddharthkp, JoyceZhu and khiga8