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 persisting author-defined focus-ring classes #36

Merged
merged 1 commit into from
Jun 13, 2017

Conversation

kloots
Copy link
Contributor

@kloots kloots commented Jun 6, 2017

This PR addresses issue #35, removing support for persistence of instances of author-add focus-ring classes.

@marcysutton
Copy link

I'm a little confused about how to use this polyfill. Can you also update the documentation for the current usage?

@kloots
Copy link
Contributor Author

kloots commented Jun 7, 2017

@marcysutton you'd simply install it as a project dependency via npm and then include the built dist as part of your rollup.

@marcysutton
Copy link

Sorry, I meant the selectors necessary to use the polyfill. I got a bit turned around by the discussion regarding CSS classes and how they should be avoided because developers will grow attached to them, and I assumed at first this change would have an impact on usage.

In practicality, though, the .focus-ring class seems required until UAs actually implement :focus-ring.

@jonathantneal
Copy link
Contributor

I think going further in the direction of attributes is actually better than classes. I will pitch a PR, and I won’t be offended if it is rejected.

@alice
Copy link
Member

alice commented Jun 12, 2017

@marcysutton To try and clear up your understandable confusion:
This concerns an (admittedly poorly documented, that's on me) option to allow developer to opt in to always matching .focus-ring by simply adding a focus-ring class to any element which should always match .focus-ring.

This concept in general is something we need, and this seemed like a good option for the polyfill since we figured it would be something developer would attempt. It's an open question right now as to what the standards-based option for this might look like.

@alice alice merged commit 25cc6d9 into WICG:master Jun 13, 2017
alice pushed a commit to alice/focus-ring that referenced this pull request Jun 18, 2017
…ing instances of author-added focus-ring class (WICG#36)"

This reverts commit 25cc6d9.
alice pushed a commit that referenced this pull request Jun 22, 2017
* Revert "Update advice on manually adding `.focus-ring`."

This reverts commit 47c0937.

* Revert "Remove use of the data-focus-ring-added as a means of persisting instances of author-added focus-ring class (#36)"

This reverts commit 25cc6d9.
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.

4 participants