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

Conversation

@joe-watkins
Copy link

@joe-watkins joe-watkins commented Mar 25, 2017

Hi :)

I've tweaked the Polyfill to reach back a bit further with IE.

  1. Added .gitignore file
  2. Defined HTML class and attribute as var: focus-ring, data-focus-ring-added
  3. Went with indexOf() instead of .contains('focus-ring')
  4. Created _addClass() & _removeClass() helper functions to replace el.classList.add() & el.classList.remove()
  5. Killed the .matches(':focus') method on line 58 -- no old IE support and felt redundant.
  6. Cleaned up HTML in demo.. something in there was causing old IE to freak out. Added doctype, and IE meta.

_addClass = function(el, htmlClass) {
el.className += ' '+htmlClass;
},
_removeClass = function(el, htmlClass) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you know the support level of RegExp in older IE? It might be nicer to do something like:

this.element.className = this.element.className.replace(regExp(name), '');

based on the polyfill example in https://developer.mozilla.org/en-US/docs/Web/API/Element/classList#Polyfill

Copy link
Author

Choose a reason for hiding this comment

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

Good call! I've snagged the regex helper function from the classList Polyfill and followed their lead. Feels cleaner :) (And because I'll never understand Regex :) ha!)

}
return triggers;
},
_addClass = function(el, htmlClass) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

because we're declaring these inside of a function I don't think you need to underscore their names

Copy link
Author

Choose a reason for hiding this comment

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

Sounds good - I've removed those underscores :)

document.body.addEventListener('keydown', function() {
hadKeyboardEvent = true;
if (document.activeElement.matches(':focus')) {
if (document.activeElement) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change seems ok to me. cc @alice for a 2nd opinion

@yoavweiss
Copy link
Contributor

Hey @joe-watkins, could you join the WICG in order to appease the IPR bots? Thanks!

@yoavweiss
Copy link
Contributor

@joe-watkins potentially helpful link: https://www.w3.org/community/wicg/participants

@joe-watkins
Copy link
Author

@robdodson This branch is way off from from gh-pages, different worlds,.. and merge conflicts are pretty gnarly. I've signed up over at WICG (tnx @yoavweiss !) which may help w/future PRs. I'm going to pull latest gh-pages and inject the IE changes from that and create a new PR.

@joe-watkins joe-watkins closed this Apr 2, 2017
@joe-watkins joe-watkins deleted the joewatkins/oldIESupport branch April 2, 2017 01:36
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.

3 participants