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

[Stylelint Plugin] Add rule to discourage use of :focus pseudo class. #359

Closed
wants to merge 3 commits into from

Conversation

mrcthms
Copy link

@mrcthms mrcthms commented Jan 23, 2023

Description

Fixes https://github.com/Shopify/web/issues/82233

This PR adds a new Stylelint rule for our CSS, which will emit an error if a developer tries to use the :focus CSS pseudo class. We want to encourage use of :focus-visible in it's place, which maintains the essential focus states for users navigating without using a traditional pointer device, but does not show the focus states on certain elements such as links and buttons for users navigating with traditional pointer devices.

We have already made a change to Polaris and Web to introduce this change, but we want to ensure folks aren't adding back in :focus declarations.

@mrcthms mrcthms requested a review from BPScott January 23, 2023 10:19
Copy link
Member

@BPScott BPScott left a comment

Choose a reason for hiding this comment

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

Not so much "please make changes", more "I don't think this is a good idea in the first place as it has a negative impact on supported browsers"

:focus-visible was added to safari in 15.4 per caniuse. Web requires support for iOS safari v12.4 because of its support for older point-of-sale devices. We might soon be able to bump that to v13.0, but that’s still well before focus-visible support.

Forcing the removal of :focus without any replacement in iOS Safari 12.4 -> 15.4 seems like it would severely impact the user experience on those older devices, which seems like a very bad idea. It would very strongly advise not doing this at all.


Do you intend for this rule to be enforced across all repositories that currently use @shopify/stylelint-plugin? Or would you want this to just be focused on admin in the web repo? If you'd want this focus to be just on the admin then I'd suggest you consider creating a rule within the web repo's internal stylelint plugin rather than pushing this change onto all consumers.

This is a very high impact rule that would require lots of refactoring across to achieve compliancy. Enabling it by default would warrant a major version bump if this were ever to be merged.

@mrcthms
Copy link
Author

mrcthms commented Feb 2, 2023

Closing in favour of https://github.com/Shopify/web/pull/83179

@mrcthms mrcthms closed this Feb 2, 2023
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