Document no href in link

[maxwondercorn]

The latest template linter has a rule requiring <a> tags have a href which breaks fileUpload. It's a pretty subtle error and if you don't test your upload function you will probably ship a bug to production 🤦

This should be documented along with how to disable the linting rule in the template file.

{{!-- template-lint-disable link-href-attributes --}}

[Alonski]

@MelSumner Pinging you to ask if this is an a11y issue?

[jelhan]

Does the addon render a <a> element itself? If I didn't missed something this is only a documentation issue, isn't it? I think the link in usage example is missing a role=button.

[MelSumner]

@Alonski yes, it is. If it goes to a URL, it should be a link. If it toggles something, submits a form, etc, then it should be a button.

[maxwondercorn]

Yes only a documentation issue. Even if the tag has role=button it's flagged by the linter

[gilest]

I just checked the documentation of the link-href-attributes rule.

Maybe we should use <button> tags in all of our examples.

[maxwondercorn]

@gilest Using a button generates an assert error for invalid tag type. Our link was stylized as a button so that was the first thing I tried.

The assert is in file-upload/component.js. I don't understand why this particular list was specified...

if (DEBUG) {
  const VALID_TAGS = ['a', 'abbr', 'area', 'audio', 'b', 'bdo', 'br', 'canvas',
    'cite', 'circle', 'clipPath', 'code', 'command', 'datalist', 'del', 'dfn',
    'em', 'embed', 'i', 'iframe', 'img', 'kbd', 'line', 'mark', 'mask', 'math',
    'noscript', 'object', 'q', 'radialGradient', 'rect', 'ruby', 'samp',
    'script', 'small', 'span', 'strong', 'sub', 'sup', 'svg', 'time', 'var',
    'video', 'wbr', 'path', 'polygon', 'polyline',
    'g', 'use'];

  component.reopen({
    didInsertElement() {
      let id = get(this, 'for');
      assert(`Changing the tagName of FileUpload to "${get(this, 'tagName')}" will break interactions.`, get(this, 'tagName') === 'label');
      let elements = this.element.querySelectorAll('*');
      for (let i = 0; i < elements.length; i++){
        let element = elements[i];
        if (element.id !== id &&
            VALID_TAGS.indexOf(element.tagName.toLowerCase()) === -1) {
          assert(`"${element.outerHTML}" is not allowed as a child of FileUpload.`);
        }
      }
    }
  })
}

[gilest]

@gilest Using a button generates an assert error for invalid tag type. Our link was stylized as a button so that was the first thing I tried.

The assert is in file-upload/component.js. I don't understand why this particular list was specified...

I think 🤔 it was to stop developers from putting block elements inside of the <label> element.

I had a quick look on MDN and:

Don't place interactive elements such as anchors or buttons inside a label. Doing so makes it difficult for people to activate the form input associated with the label.

Let's reconsider the markup of this addon entirely so that it meets modern HTML and accessibility standards ✅

[gilest]

@maximilianmeier I don't see a reason why button couldn't be added to the VALID_TAGS array in the meantime... Thoughts?

[oliverlj]

I use a span with role="button". but my first choice was button which is not available

[maximilianmeier]

I don't see any problems with the button tag on first sight

[raido]

I used span role=button approach as well while fixing template lint errors.

[Sticksword]

+1 should be able to use button, also ran into this recently!

[gilest]

In v5 we are deprecating the component in favour of a modifier.

See the Upgrade guide here.

Feedback appreciated.

[gilest]

Should be resolved by v5