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

Remove support for author-added focus-ring class #35

Closed
kloots opened this issue Jun 6, 2017 · 6 comments
Closed

Remove support for author-added focus-ring class #35

kloots opened this issue Jun 6, 2017 · 6 comments

Comments

@kloots
Copy link
Contributor

kloots commented Jun 6, 2017

Filing up as a followup from the discussion in issue #33 pertaining to the need for the data-focus-ring-added attribute. As of 8d00580 this polyfill supports persistence of instances of an author-defined focus-ring class. I would argue this should be removed on the premise that:

  1. This polyfill shouldn't give authors abilities they won't have with the actual :focus-ring psuedo. Therefore, once actual support for :focus-ring lands in browsers the only change authors need to make to their CSS is changing their rules from .focus-ring to :focus-ring. Hence, more with the grain.

  2. Should authors wish to ensure specific instances of UI always show focus they can leveraging the natural cascade and/or specificity rules in CSS:

// hide focus by default
:focus {
  outline: none;
}
// focus ring for keyboard navigation
.focus-ring {
  outline: solid 2px blue;
}

// always show focus outline around a specifc control
button.default-action:focus {
  outline: solid 2px blue;
}
@alice
Copy link
Member

alice commented Jun 6, 2017

The issue with this:

// always show focus outline around a specifc control
button.default-action:focus {
  outline: solid 2px blue;
}

is that there is no way to recover the default focus style.

@kloots
Copy link
Contributor Author

kloots commented Jun 6, 2017

@alice you didn't address my original point, which is once support for the actual :focus-ring pseudo lands in browsers, how would authors achieve the persistence you are currently offering in the polyfill? It doesn't seem like a good idea for this polyfill to temporarily give authors an ability they eventually won't have.

That aside, authors wishing to restore the browser's default focus style could apply a class to the html element identifying the UA, hide the focus ring by setting the outline-width to 0 and restore it by setting the outline-width to the correct default width per UA. Here's an example:

/* Hide focus ring by default */
:focus {
  outline-width: 0;
}

/* Show focus ring when navigating via keyboard */
.moz .focus-ring {
  outline-width: medium;
}

.chrome .focus-ring {
  outline-width: 5px;
}

/* Force focus ring to always be visible */
.moz .default-action:focus {
  outline-width: medium;
}

.chrome .default-action:focus {
  outline-width: 5px;
}

@jonathantneal
Copy link
Contributor

We can restore outline via outline-width in native land:

:not(:focus-ring):focus {
  outline-width: 0;
}

.some-override:focus {
  outline-width: medium;
}

Removing non-spec syntactical sugar will make developer’s lives easier later. In my experience, they will always come to rely on your non-spec sugar.

@robdodson
Copy link
Collaborator

alice and I discussed this and agree that we can remove the attribute and update the recommendations to use outline-width. It would be good to figure out a standards based way to add elements to the "always match focus ring behavior" but we can take that up as a separate issue. We would probably want an additional CSS property for something like that.

@alice
Copy link
Member

alice commented Jun 18, 2017

I added an issue to discuss the "always match focus ring" thing: #42

@robdodson
Copy link
Collaborator

I think this issue was resolved with #36. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants