Extract HTML <datalist> input type subfeatures#28189
Conversation
|
Tip: Review these changes grouped by change (recommended for most PRs), or grouped by feature (for large PRs). |
360ac12 to
3d54188
Compare
caugner
left a comment
There was a problem hiding this comment.
Note to myself: Missing spec URLs.
Elchi3
left a comment
There was a problem hiding this comment.
and duplicates these below
html.input.elements.type_*.
I'm not sure if this is needed, is it?
I would argue that the duplication is helpful for web developers that are looking at the input types. Recently, we used a similar approach for CSS transition, and IIRC we have a few related cases where a BCD feature relates to two APIs features. In this case, the duplication helped me ensure the data is correct. I could imagine adding a Edit: That said, if you prefer that I delete the duplicates, I can drop them. |
|
I would like a second opinion if the duplication makes sense. Maybe @ddbeck? |
There was a problem hiding this comment.
I don't love the duplication, though I can kinda-sorta see a way of doing it that is more idiomatic:
The subfeatures of the html.elements.input.* features ought to be named list, representing support for the list attribute on that type. Since we treat the input types as if they were different elements, we can represent support for list as if it was an attribute unique to each input type (i.e., act as though <input type="color"> is <input-color>, then the subfeature is <input-color list="…">.
In this scenario, the duplication would be at the point of html.elements.datalist.input_type_*. Since <datalist> is the new thing, I think duplicating this data on html.elements.datalist makes sense as a convenience to MDN readers.
If we do this, I think there's a case to be made to remove html.elements.input.list as needless duplication (since it would be a duplicate of html.elements.input.text.list) and confusing (does it mean support for all types? At least one? Something else?).
(This makes me wish we had some sort of annotation in the data, something like canonical: "html.elements.input.color.list" on keys that duplicate other keys.)
html/elements/datalist.json
Outdated
| "version_added": "4", | ||
| "version_removed": "110", | ||
| "partial_implementation": true, | ||
| "notes": "The `<datalist>` element will only create a dropdown for textual types, such as `text`, `search`, `url`, `tel`, `email` and `number`. The `date`, `time`, `range` and `color` types are not supported." |
There was a problem hiding this comment.
Not all of the types mentioned in the note are represented by the subfeatures. The PR title had me anticipating keys like this:
- html.elements.datalist
- html.elements.datalist.input_type_color
- html.elements.datalist.input_type_date
- html.elements.datalist.input_type_email
- html.elements.datalist.input_type_number
- html.elements.datalist.input_type_range
- html.elements.datalist.input_type_search
- html.elements.datalist.input_type_tel
- html.elements.datalist.input_type_text
- html.elements.datalist.input_type_time
- html.elements.datalist.input_type_url
There was a problem hiding this comment.
This PR only extracts these four from existing data:
- html.elements.datalist.input_type_color
- html.elements.datalist.input_type_date
- html.elements.datalist.input_type_range
- html.elements.datalist.input_type_time
The current data suggests to me the following textual types were always supported:
- html.elements.datalist.input_type_email
- html.elements.datalist.input_type_number
- html.elements.datalist.input_type_search
- html.elements.datalist.input_type_tel
- html.elements.datalist.input_type_text
- html.elements.datalist.input_type_url
There was a problem hiding this comment.
To clarify, this PR only extracts subfeatures for types currently mentioned data, without researching/collecting/adding data for other types not currently mentioned.
There was a problem hiding this comment.
OK. I won't stand in the way of merging on this, but I would like to see an issue filed to complete the missing data.
(I would still like @Elchi3's review before I approve the PR overall, as it does imply some things that are not in the PR and it'd be nice to have consensus before proceeding in an incomplete state.)
There was a problem hiding this comment.
Yeah, seems sad that this PR doesn't add the complete list of input types support. The issue is filed, though, so I won't stand in the way either.
|
@Elchi3 Please take a look at this PR when you have a moment. It would be nice to land this change. 🙏 |
|
@ddbeck If you find time to review this, that would be lovely. 🙏 |
|
This pull request has merge conflicts that must be resolved before it can be merged. |
aca15ce to
dbf1f5d
Compare
dbf1f5d to
06bb9c6
Compare
|
Merged As previously mentioned, I will tackle the remaining missing types separately. |
okay, this convinced me. |
Summary
Extracts HTML
<datalist>input type subfeatures, and duplicates these belowhtml.input.elements.type_*.Test results and supporting details
To review, look at "grouped by change" in this comment.
Related issues
Fixes #17480.
Once landed, we can tackle the issues mentioned in #25723.