V1 custom elements - missing functionality

[treshugart]

I took the V1 custom element polyfill and plopped it into SkateJS. The plan here is to exclusively use v1 in Skate (maybe fallback to v0, but that's a big maybe). It's allowed us to remove lots of code, but I ran into a few issues where there was missing functionality.

The purpose of this issue is more to document and reference the PR that I will raise for it.

The functionality we've required that will be in the PR is:

• CustomElementRegistry.prototype.get()
• Calling CustomElement.prototype.attributeChangedCallback() for existing attributes in part 8 of the upgrade process

There's a couple other things I found missing while passing over the code. We don't require these right away so I'll leave them out of the PR.

• CustomElementRegistry.prototype.whenDefined()
• Patching all built-in HTMLElement subclasses. I was going to do this, but ended up not doing it as the way to do this is still contentious in that Apple is pretty hard-fast about not using the is attribute. Therefore Skate will hold off on doing this as well for its v1.

[ebidel]

BTW, dos anyone know why is="" is being debated? Seems like a huge win to support extending native elements (dev ergonomics, extending platform primitives, writing less code, inherit a11y / other benefits, etc.).

A super button becomes:

class BetterButton extends HTMLButtonElement {
  constructor() {
    super();

    this.addEventListener('click', e => {
      this.drawAnimation(e.offsetX, e.offsetY);
    });
  }

  drawAnimation(x, y) {
    ...
  }
}

instead of:

class BetterButton extends HTMLElement {

  static get observedAttributes() {
    return ['disabled'];
  }

  get disabled() {
    return this.hasAttribute('disabled');
  }

  set disabled(val) {
    if (val) {
      this.setAttribute('disabled', '');
    } else {
      this.removeAttribute('disabled');
    }
  }

  constructor() {
    super();

    this.addEventListener('keydown', e => {
      if (e.keyCode === 32 || e.keyCode === 13) {
        this.dispatchEvent(new MouseEvent('click', {
          bubbles: true,
          cancelable: true
        }));
      }
    });

    this.addEventListener('click', e => {
      if (this.disabled) {
        e.preventDefault();
        e.stopPropagation();
      }
      this.drawAnimation(e.offsetX, e.offsetY);
    });
  }

  connectedCallback() {
    this.setAttribute('role', 'button');
    this.setAttribute('tabindex', '0');
  }

  attributeChangedCallback(name, oldValue, newValue) {
    if (this.disabled) {
      this.setAttribute('tabindex', '-1');
      this.setAttribute('aria-disabled', 'true');
    } else {
      this.setAttribute('tabindex', '0');
      this.setAttribute('aria-disabled', 'false');
    }
  }

  drawAnimation(x, y) {
    ...
  }
}

... a lot of needless bookkeeping for developers.

cc @justinfagnani

[treshugart]

@ebidel I was referring to WICG/webcomponents#509 (comment) specifically. I think they're in favour of using your first example, it's just that the consumer of a component shouldn't have to know about is.

[justinfagnani]

@treshugart The spec changed a bit since I last worked on the polyfill, so there are definitely new-ish things that are missing, as well as some known stuff from the first sprint. I think this is mostly get and whenDefined on CustomElementsRegistry: https://html.spec.whatwg.org/multipage/scripting.html#customelementsregistry

Regarding upgrades and existing attributes, that should be implemented here: https://github.com/webcomponents/webcomponentsjs/blob/v1/src/CustomElements/v1/CustomElements.js#L240 thought it looks like there isn't test coverage yet, so it's possible it's broken.

The comment in the code about Patching all built-in HTMLElement subclasses has nothing to do with is= and needs to be done to make new HTMLDivElement() instanceof HTMLElement return true. Since we patch the HTMLElement constructor, the HTMLElement in that expression is not the same as the one on the prototype chain of the built-ins.

Note about specs: the DOM and HTML specs are the canonical specs now, so to make sure we aren't referencing something outdated, let's link to those.

[treshugart]

Regarding upgrades and existing attributes, that should be implemented here: https://github.com/webcomponents/webcomponentsjs/blob/v1/src/CustomElements/v1/CustomElements.js#L240 thought it looks like there isn't test coverage yet, so it's possible it's broken.

That line only observes attribute changes. If any attributes exist before that is called, then attributeChangedCallback won't be triggered for those. I could be interpreting the spec incorrectly, but I'm referring to part 1 here: https://html.spec.whatwg.org/multipage/scripting.html#upgrades.

Note about specs: the DOM and HTML specs are the canonical specs now, so to make sure we aren't referencing something outdated, let's link to those.

👍

[justinfagnani]

Ah, gotcha. That should be an easy fix.

One big thing that's come up is that it looks we're going to need to support CE polyfill in environments that have native Shadow DOM. I'm planning on another sprint of work to get that, HTML Imports support, and more tests in.

To avoid any duplicate work, it's probably best to make issues for each feature and accept it as we work on it.

[treshugart]

Totes. I'll close the PR and this issue, and then separate them. I'm more
than happy to own the ones I raise from this. I also wanted to ask a couple
of things:

The v1 tests don't seem to be running right now. Is there a reason for
that? Should this polyfill override the v0 polyfill and get run in its
place?

It seems like ES2015 classes were used, but other features weren't. What's
the convention here? Can I go full ES2015 with the code I'm adding? What
are the transpilation plans here?

In terms of native Shadow DOM, it seemed to work alongside V0 SD just fine,
but you're probably referring to features I wasn't using or integrations
with V1.

On Wed, 8 Jun 2016, 03:25 Justin Fagnani [email protected] wrote:

Ah, gotcha. That should be an easy fix.

One big thing that's come up is that it looks we're going to need to
support CE polyfill in environments that have native Shadow DOM. I'm
planning on another sprint of work to get that, HTML Imports support, and
more tests in.

To avoid any duplicate work, it's probably best to make issues for each
feature and accept it as we work on it.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#551 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAIVbO6HNho_ss3RbnkpXhrvPZg3etwQks5qJalsgaJpZM4Iufv6
.

[treshugart]

Split each change out into #554, #555, #556 and #557.

[justinfagnani]

@treshugart thanks for the Issue/PR split.

I've been running the tests via this command:

wct tests/CustomElements/v1/runner.html -l chrome

But I can wire them in so they'll run by default.

This was all WIP that I paused when the spec continued to change under me. Glad you're helping out now!