这是indexloc提供的服务，不要输入任何密码
Skip to content

Add a focusring custom event #30

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Add a focusring custom event #30

wants to merge 3 commits into from

Conversation

@kloots
Copy link
Contributor

@kloots kloots commented May 31, 2017

This PR adds a focusring custom event, triggered on the element to which the focus-ring class is to be applied. This allows projects further control over when a focus outline is rendered for a given element.

For example, at Slack we're using the focus-ring library but prefer to only see a focus outline when the user is navigating via the keyboard using either Tab or Shift + Tab. With this change we'd be able to add a single, document-level listener to manage this:

document.addEventListener('focusring', function(e) {
  if (e.detail.srcEvent.keyCode != 9) {
    e.preventDefault();
  }
}, true);

@alice
Copy link
Member

alice commented Jun 2, 2017

Hm, I don't think this belongs in the polyfill - this behaviour is not specced, and I don't think it could be specced for a pseudo-selector.

@kloots
Copy link
Contributor Author

kloots commented Jun 2, 2017

@alice i'm not adding a behavior, just adding an event. What troubles you about adding an event?

@kloots
Copy link
Contributor Author

kloots commented Jun 2, 2017

@alice the only alternatives I could see for addressing this bug were:

  1. Changing the polyfill such that only Tab and Shift + Tab keypress result in focus-ring being applied—since by default in the browser those are the only keys that result in focus anyway.
  2. Allowing for the polyfill to be initialized with a config so people can supply the keys that should result in the focus-ring class being applied.

@kloots
Copy link
Contributor Author

kloots commented Jun 2, 2017

All things considered, providing an event feels the most with-the-grain of how existing DOM works.

@alice
Copy link
Member

alice commented Jun 2, 2017

Firing the event is the behaviour I was referring to. What troubles me is that this is intended as a polyfill for a pseudo-selector, and pseudo-selectors don't fire events when they get matched.

I don't think this is a bug per se - it's a case of desiring different behaviour from the pseudo-selector. The specifics of when :focus-ring should match are specifically left vague, so that browsers may choose what they believe is best for their users (so in any case, using native :focus-ring would never give you this level of control). Currently, this polyfill matches the behaviour of a native <button> element.

I filed #33 to continue this discussion - could you comment there about why you prefer this matching behaviour?

@kloots
Copy link
Contributor Author

kloots commented Jun 5, 2017

Closing this PR as I've addressed the need for this solution in PR #34 after careful consideration with @alice.

@kloots kloots closed this Jun 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kloots @alice