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

Generator: focus-visible nested polyfill support #113

Merged
merged 4 commits into from
May 17, 2021

Conversation

blackfalcon
Copy link
Member

@blackfalcon blackfalcon commented May 7, 2021

Alaska Airlines Pull Request

This PR will add new documentation and demo support for how the focus-visible pseudo class works when natively supported and when the polyfill is in use.

It has been discovered that the process from which to support nested elements and the polyfill is causing other issues and will not be included within the scope of this PR.

Resolves: #69

Type of change:

Please delete options that are not relevant.

  • New capability
  • Revision of an existing capability
  • Infrastructure change (automation, etc.)
  • Other (please elaborate)

Checklist:

  • My update follows the CONTRIBUTING guidelines of this project
  • I have performed a self-review of my own update

By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.

Thank you for your submission!

-- Auro Design System Team

@blackfalcon blackfalcon requested a review from geoffrich May 7, 2021 03:36
@blackfalcon blackfalcon self-assigned this May 7, 2021
@blackfalcon blackfalcon force-pushed the dsande/focus-visibleDeprecation/#69 branch from 19ecb02 to 389c95b Compare May 7, 2021 03:42
@blackfalcon blackfalcon linked an issue May 7, 2021 that may be closed by this pull request
docs/focus-visible.md Outdated Show resolved Hide resolved
template/src/[namespace]-[name].js Show resolved Hide resolved
docs/focus-visible.md Outdated Show resolved Hide resolved
docs/focus-visible.md Outdated Show resolved Hide resolved
docs/focus-visible.md Outdated Show resolved Hide resolved
@geoffrich
Copy link
Contributor

Going to investigate further on Monday, but it seems that JSDOM throws an error when the polyfill is applied to a shadow root. I want to see if there's a way to mitigate this before adding it to the generator.

@blackfalcon blackfalcon added the Status: Blocked Progress on the issue is blocked, immediate attention is required label May 10, 2021
@blackfalcon blackfalcon force-pushed the dsande/focus-visibleDeprecation/#69 branch from 389c95b to 27594e2 Compare May 10, 2021 21:43
@blackfalcon
Copy link
Member Author

Marked this issue as BLOCKED until I hear back from @geoffrich

Going to investigate further on Monday...

@geoffrich
Copy link
Contributor

Working on it. Trying to make a minimal JSDOM repro.

@geoffrich
Copy link
Contributor

Okay, turns out this is not a false alarm. Loading the polyfill in the constructor applies an attribute to the element, and this is a violation of the spec for custom element constructors:

The element must not gain any attributes or children, as this violates the expectations of consumers who use the createElement or createElementNS methods.

I think we need to move the polyfill loading to connectedCallback instead.

@blackfalcon I can apply the changes first in auro-button and then to this WC-Generator branch once everything works. I should be able to complete that work tomorrow.

@geoffrich
Copy link
Contributor

See AlaskaAirlines/auro-button#112 for the associated button issue.

@blackfalcon
Copy link
Member Author

I am thinking that the document update stays, but the code will not. And the days of supporting this polyfill are numbered.

@blackfalcon blackfalcon force-pushed the dsande/focus-visibleDeprecation/#69 branch from 27594e2 to f5081d2 Compare May 11, 2021 18:38
@blackfalcon blackfalcon force-pushed the dsande/focus-visibleDeprecation/#69 branch 4 times, most recently from a4bfb91 to 6fee1fc Compare May 14, 2021 23:20
@blackfalcon blackfalcon requested a review from geoffrich May 14, 2021 23:24
@blackfalcon blackfalcon removed the Status: Blocked Progress on the issue is blocked, immediate attention is required label May 14, 2021
Base automatically changed from next to master May 17, 2021 15:17
@blackfalcon blackfalcon force-pushed the dsande/focus-visibleDeprecation/#69 branch 2 times, most recently from 1f4248b to ae93333 Compare May 17, 2021 18:55
@blackfalcon
Copy link
Member Author

@geoffrich the scope of this PR has been reduced to only document and style changes. The issue related to nested elements is still an open discussion.

@blackfalcon blackfalcon force-pushed the dsande/focus-visibleDeprecation/#69 branch from ae93333 to 4fca9e9 Compare May 17, 2021 18:59
docs/focus-visible.md Outdated Show resolved Hide resolved
@blackfalcon blackfalcon force-pushed the dsande/focus-visibleDeprecation/#69 branch from 6ff900b to d984b9f Compare May 17, 2021 22:23
@blackfalcon blackfalcon merged commit 3d57fcd into master May 17, 2021
@blackfalcon blackfalcon deleted the dsande/focus-visibleDeprecation/#69 branch May 17, 2021 22:44
@AuroDesignSystem
Copy link
Collaborator

🎉 This PR is included in version 3.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@AuroDesignSystem AuroDesignSystem added the released Completed work has been released label May 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Completed work has been released Type: Feature New Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

generator: focus-visible support update
3 participants