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

Form-associated custom elements missing features #5016

Open
annevk opened this issue Oct 17, 2019 · 30 comments
Open

Form-associated custom elements missing features #5016

annevk opened this issue Oct 17, 2019 · 30 comments
Labels
accessibility Affects accessibility addition/proposal New features or enhancements topic: custom elements Relates to custom elements (as defined in DOM and HTML) topic: forms

Comments

@annevk
Copy link
Member

annevk commented Oct 17, 2019

I was looking at pseudo-classes and it seems that FACEs miss a couple of features:

  • The ability to be required
  • The ability to be read-write
  • The ability to be in a show a placeholder state
  • The ability to be the default submit button (I think we track this one)
  • The ability to be the default as part of a group or designate one of the items it consists of as the default (<input type=checkbox checked> and <option selected> match :default)
  • The ability to be indeterminate
  • The ability to be active (see also Activation behavior of label elements pointing to form-associated custom elements #5009)
@annevk annevk added addition/proposal New features or enhancements topic: forms labels Oct 17, 2019
@domenic
Copy link
Member

domenic commented Oct 17, 2019

/cc @tkent-google. I think required might be already integrated but I am not sure.

In general it's unclear how much here is complicated underlying semantics vs. just allowing elements to set booleans that map directly to psuedo classes. Thus @tkent-google's original suggestion that you could set built-in pseudoclasses with ElementInternals, but we made that a "v2" feature to focus on :state() first.

@annevk
Copy link
Member Author

annevk commented Oct 17, 2019

:required is not defined to match FACEs.

readonly attribute does impact willValidate at the moment for FACEs, but does not impact pseudo-classes.

I think some of this is also exposed to a11y trees in which case a semantic boolean would be better for developers I think.

@annevk annevk added the accessibility Affects accessibility label Oct 17, 2019
@annevk
Copy link
Member Author

annevk commented Oct 17, 2019

cc @whatwg/a11y

@tkent-google
Copy link
Contributor

This is a dupe of WICG/webcomponents#813 and WICG/webcomponents#814.

@annevk
Copy link
Member Author

annevk commented Oct 17, 2019

I think it only is if you do not consider the a11y tree. (Which is why it's annoying that's not specified in HTML, as it's easily overlooked.) But also, I think we should also track such ideas here as it affects HTML in the end.

cc @whatwg/forms

@aardrian
Copy link

aardrian commented Oct 17, 2019

@annevk

(Which is why it's annoying that's [a11y tree] not specified in HTML, as it's easily overlooked.)

While your parenthetical statement is orthogonal to this issue, is the HTML Accessibility API Mappings not providing what you need? Should WHATWG's HTML spec provide a prominent link so it is not overlooked?

@annevk
Copy link
Member Author

annevk commented Oct 17, 2019

That would certainly help and is being tracked by #3282.

I ended up filing w3c/html-aam#257 for readonly/disabled, as a start.

@domenic
Copy link
Member

domenic commented Oct 19, 2019

The distinction between :disabled/:enabled (which work for FACEs) and :read-only/:read-write (which does not) is an interesting one. See w3c/html-aam#257 (comment) for the a11y side of it; here let's discuss the CSS side.

My impression is that, because the "default path" for FACE authors is to have non-useful readonly behavior, @tkent-google set up the current design to have them not match the CSS pseudo-classes. Instead, the vision was that we would use some future mechanism (like WICG/webcomponents#813) to allow authors to manually set up the pseudo-classes while they were also setting up the rest of their control-specific readonly behavior.

This still seems reasonable to me, but I am open to changing it. It seems like the other tenable position is that readonly="" should give as much readonly behavior as possible, including CSS pseudo-class matching, even if the FACE author still has to implement the actual readonly-ness. The failure mode, i.e. controls which the user can still edit but which match :read-only, seems a bit worse than the current design. But maybe we shouldn't be designing so much around authors who forget to implement readonly="" support?

@tkent-google
Copy link
Contributor

It seems like the other tenable position is that readonly="" should give as much readonly behavior as possible, including CSS pseudo-class matching, even if the FACE author still has to implement the actual readonly-ness.

UAs need to detect whether a FACE is text-editable or not to detect :read-write state if the FACE has no readonly attribute. I think API like internals.isEditable = true internals.supportsReadWritePseudoClass = true is necessary.

@annevk
Copy link
Member Author

annevk commented Oct 21, 2019

I think a better default for form controls is that they are editable. And also, from its current definition it seems that :read-write is only undone by disabled or readonly, which are both supported by a FACE (this abbreviation is the worst by the way, but I also kinda like it). (Also seems that Firefox might not support :read-write, investigating that separately.)

@domenic
Copy link
Member

domenic commented Oct 21, 2019

See #3501 for the interop issues with :read-write/:read-only. It does seem like we need to straighten that out before making any changes for FACEs, because it might change the underlying model. For example it seems like "is text-editable" is relevant to the Blink semantics, but not the WebKit/spec semantics. Or, we might even consider removing the pseudo-classes altogether, if usage data shows them being low.

@annevk annevk added the topic: custom elements Relates to custom elements (as defined in DOM and HTML) label Oct 24, 2019
@dflorey
Copy link

dflorey commented May 4, 2020

Cross-posting here as this seems to be the right place.

When calling reportValidity() on a native input field it will display the error message using the built-in UI (e.g. error popup on Chrome).
In my custom element I'd implement reportValidity() in a way that matches the design guidelines (e.g. material design).
I'd assume that when submitting a form on a form-associated custom element my reportValidity() would be called instead of the built-in UI, but unfortunately this seems not to be the case.

In the given sample in the doc...

https://docs.google.com/document/d/1JO8puctCSpW-ZYGU8lF-h4FWRIDQNDVexzHoOQ2iQmY/edit?pli=1#heading=h.2izmbauztyja

....and in all other examples that I've found it is also a bit weird that the validity will be set whenever the input changes as this is may be a very expensive operation.
Think of a signature input fields that does all kinds of image analysis to check if the control is valid. So the validation should only be checked when the browser calls my checkValidity() or reportValidity() methods instead on each change of the input.

I'd also expect to have a formDataCallback(FormData) or something along those lines to add the contents of the custom element to the form data to be submitted.
Right now the only way is to call this.internals_.setFormValue('') to set the value, but when should I call this method? Whenever the value changes? Again this may take a lot of time to calculate the value based on the input, so I'd want to call this method only whenever the user submits the form. How to do that?
I tried to use the event based form participation:

https://docs.google.com/document/d/1JO8puctCSpW-ZYGU8lF-h4FWRIDQNDVexzHoOQ2iQmY/edit?pli=1#heading=h.je8c7y5qpgki

but this seems not to work (quite yet)?

It would be so cool to be able to implement a custom element that can be used as a drop-in replacement for native input fields or to add new exciting input controls, but right now I don't see a way to do so due to the issues above.
Hopefully I just got it all wrong. I'd be super-happy to find a code example that shows a basic example how to

  • add the value to the formdata on form submission
  • performs the validation check when the form gets validated (=user clicks on submit)
  • reports an error in a custom way and avoids the default browser UI

@annevk
Copy link
Member Author

annevk commented May 4, 2020

Can you not use the invalid event?

@dflorey
Copy link

dflorey commented May 5, 2020

Yes, I could fire an invalid event just like native input elements - once my reportValidation() method would get invoked. But it is never called.
I guess once the MWC team will try to implement proper validation they will come across the same issues:
material-components/material-web#971

@annevk
Copy link
Member Author

annevk commented May 5, 2020

Sorry, I think the way this works is that you use setCustomValidity() from oninput or some such. (Note that builtin elements also validate on the fly. Performing validation during submission only would not be proper.) If that isn't sufficient a new issue detailing your use case would help.

@dflorey
Copy link

dflorey commented May 5, 2020

Thanks, will do.
Form my understanding after reading the validation API spec setCustomValidity() is meant to be used by component users, not component authors and that makes perfect sense to me.
IMO component authors should implement reportValidity() , checkValidity() etc. as only the component knows how to report the validation properly and how to check the validity.

@annevk
Copy link
Member Author

annevk commented May 5, 2020

Component developers would not override the public API. They would have some kind of callback or something on ElementInternals. Edit: and there is such a thing, setValidity(). I don't really understand what the problem is here now.

@dflorey
Copy link

dflorey commented May 5, 2020

As stated above the problem is that there is no way to create a custom element that
-adds the value to the formdata on form submission
-performs the validation check when the form gets validated (=user clicks on submit)
-reports an error in a custom way and avoids the default browser UI
Just try to use the mwc-textfield or any other custom element and try to use it in a form. It just does not work and never will due to the issues above.

@annevk
Copy link
Member Author

annevk commented May 5, 2020

The formdata event allows for 1. 2 is not possible but is also not an option builtin elements have. You have to know before that point if something is valid or not. 3 is possible in the same way it is for builtins (cancel the invalid event and display something), afaict.

@dflorey
Copy link

dflorey commented May 5, 2020

Thanks for the valuable info! (1) was not working for me. I'll give (3) a try. Would be great to have a working example somewhere. It would make custom elements so much more useful if that worked.

@annevk
Copy link
Member Author

annevk commented Jun 12, 2020

@domenic now that #3501 got resolved somehow, any new thoughts on that?

@rniwa
Copy link

rniwa commented Aug 12, 2020

Can someone compile a list of things we need to tackle? Maybe we can discuss it in fall.

@annevk
Copy link
Member Author

annevk commented Aug 24, 2020

@rniwa I think the list in OP is still accurate and given that it's not just pseudo-classes I rather have a solution that works for both pseudo-classes and AT/equivalent tools than requiring detailed work for each.

@scottaohara
Copy link
Collaborator

some additional attributes that need to be spec'd for FACES which aren't in the OP.

  • autocomplete
  • max
  • min

@annevk
Copy link
Member Author

annevk commented Jun 5, 2021

@domenic ping on reconsidering :read-only/:read-write.

@domenic
Copy link
Member

domenic commented Jun 7, 2021

So re-reading the thread, I guess we have a few options:

  1. Assume all FACEs "support the readonly attribute", and then do as much automatically as possible for them:

    • When readonly="" is present, signal to AT that it is readonly
    • Match :read-only when appropriate, or :read-write otherwise, depending on the readonly="" and disabled="" attributes.
    • The control author must look at this.hasAttribute("readonly") and use that to modify behavior.
  2. Assume by default a FACE does not "support the readonly attribute". If they want to explicitly support it, then they do the following:

  3. Add a single switch for supports-readonly-ness, e.g. internals.supportsReadonly = true. This gives the behaviors of (1).

(1) seems bad since it means you could not create controls like range, color, checkbox, etc. which do not support readonly="". (2) is the status quo but without WICG/webcomponents#813 these controls will (like <div>) always match :read-only and never match :read-write.

So I think our choices are between pursuing WICG/webcomponents#813 and pursuing something like (3). I worry about (3) since it seems very special-casey and we haven't heard any author demand for readonly-supporting FACEs. I'd be surprised if readonly FACEs were the top priority to add among all the other things in the OP. And I think WICG/webcomponents#813 would solve a lot of other items listed in the OP.

@domenic
Copy link
Member

domenic commented Jun 7, 2021

some additional attributes that need to be spec'd for FACES which aren't in the OP.

Note that FACEs can get correct AT behavior here for max/min by setting internals.ariaValueMin/internals.ariaValueMax according to the values of their max=""/min="" attributes, if they support those, or according to any other attributes or properties if they use a different user interface for setting value maximums/minimums.

For autocomplete, I think allowing FACE to accept autocomplete is an unsolved problem and pretty difficult given that it involves the integration of user agent UI with arbitrary author-drawn content.

@MaxArt2501
Copy link

I don't know if it's ever been mentioned, but I think FACEs are also missing:

  • the ability to have an associated datalist.

I imagine something like a ElementInternals.list property that could be set to a HTMLDataListElement, and this could react to the value set via setFormValue. Then a FACE should implement this kind of code:

class MyInput extends HTMLElement {
  static observedAttributes = ['list', /* ... */];

  // ...
  
  attributeChangedCallback(name, oldValue, newValue) {
    if (name === 'list') {
      this.#internals.list = this.getRootNode().getElementById(newValue);
    }
    // ...
  }

  get list() {
    return this.#internals.list;
  }
}

... now the tricky part: how to actually place the dropdown? For HTMLInputElements, it's placed just below them - meaning just below the box the user writes in. But for custom elements said box might not have the same size as the host, possibly resulting in an incorrect placement. I wouldn't dare to suggest to add something like ElementInternals.listOffsetX/listOffsetY, but I have no experience in this kind of problems.

The other issue is controlling when the dropdown should open. In Chromium/Safari it opens as soon as the control is focused, but not in Firefox, for example.

What do you folks think?

@domenic
Copy link
Member

domenic commented Sep 2, 2021

Hmm. I always thought of "the ability to have an associated datalist" as something very specific to text inputs that they implement themselves (similar to how they implement cursor movement and selection). But I could see an argument for it being a more general capability...

@chrisdholt
Copy link

Hmm. I always thought of "the ability to have an associated datalist" as something very specific to text inputs that they implement themselves (similar to how they implement cursor movement and selection). But I could see an argument for it being a more general capability...

Would solving this issue provide a path forward to solving the unresolved problem of autocomplete with FACE's? I'd love to see that gap resolved as the value to users is significant, but I absolutely understand (and appreciate) the difficulty as you've pointed out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Affects accessibility addition/proposal New features or enhancements topic: custom elements Relates to custom elements (as defined in DOM and HTML) topic: forms
Development

No branches or pull requests

9 participants