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.

Sign up for GitHub

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

Jump to bottom

enhance: html input element - technical summary - attributes #31639

Merged
merged 6 commits into from
Jul 11, 2024
Merged

enhance: html input element - technical summary - attributes #31639

merged 6 commits into from
Jul 11, 2024

Conversation

Malix-off
Copy link
Contributor

@Malix-off Malix-off commented Jan 10, 2024 •
edited
Loading

Description

General enhancements for docs about html input element - technical summary - attributes

Motivation

The docs about html input element - technical summary - attributes are not complete enough and well build, to me.
I think they should be done using a JSON object as representation schema, to automatically link attributes to their specific elements doc page, and list the specific elements doc page from the attribute section in the general input element doc page.
This is an attempt to make a manual update about them, albeit not being enough

Related issues and pull requests

Close #31590

Tasks
@github-actions github-actions bot added the Content:HTML Hypertext Markup Language docs label Jan 10, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 10, 2024 •
edited
Loading

Preview URLs (9 pages)
Flaws (4)

Note! 6 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/HTML/Element/input/week
Title: <input type="week">
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/HTMLInputElement/value does not exist

URL: /en-US/docs/Web/HTML/Element/input/month
Title: <input type="month">
Flaw count: 2

  • macros:
    • /en-US/docs/Web/API/HTMLInputElement/value does not exist
    • /en-US/docs/Web/API/HTMLInputElement/value does not exist

URL: /en-US/docs/Web/HTML/Element/input/range
Title: <input type="range">
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/HTMLInputElement/valueAsNumber does not exist

(comment last updated: 2024-06-24 20:54:05)
@Malix-off Malix-off mentioned this pull request Jan 10, 2024
Closed
@estelle
Copy link
Member

estelle commented Jan 10, 2024

Apologies as I didn't realize this was still in draft. commit e331252 fixes the linting issues with the table (all white space).
@Malix-off
Copy link
Contributor Author

Malix-off commented Jan 10, 2024 •
edited
Loading

@estelle np, the only reason it's in draft is for this task: #31639 (comment), which I would prefer a more proficient maintainer who is more knowledgeable about the standards of valueAsNumber
Malix-off referenced this pull request Jan 10, 2024
@Malix-off
fix: indentation
296aea0
@estelle
Copy link
Member

estelle commented Jan 10, 2024

@estelle np, the only reason it's in draft is for this task: #31639 (comment), which I would prefer a more proficient maintainer who is more knowledgeable about the standards of valueAsNumber

You don't want to add the IDL attributes on this page. Those are listed on https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement

The attributes for this page are listed on https://html.spec.whatwg.org/multipage/input.html#the-input-element under "content attributes". The DOM interface attributes go under the API/HTMLInputElement page
@Malix-off
Copy link
Contributor Author

Okay,

Should I revert the change in the html input element general doc page, and reroute the links to valueasdate and valuesasnumber to the API instead?

Also, for some reason, value and list are listed both in the IDL and the common attributes, you might want to check it
@estelle
Copy link
Member

estelle commented Jan 10, 2024

Also, for some reason, value and list are listed both in the IDL and the common attributes, you might want to check it

That's because both value and list are both HTML attributes that you add to the opening tag and IDL attributes that you can retrieve from the input object with JS.
@estelle
Copy link
Member

estelle commented Jan 10, 2024 •
edited
Loading

Should I revert the change in the html input element general doc page, and reroute the links to valueasdate and valuesasnumber to the API instead?

The valueAsDate and valueAsNumber should not be mentioned on this page at all. So, yes, please revert those additions. Thanks

That said, i am unclear why only value is listed under IDL attributes on the month input in the technical summary section. There are way more attributes than that.
@Malix-off
Copy link
Contributor Author

Malix-off commented Jan 10, 2024 •
edited
Loading

@estelle

There are way more attributes than that.

Indeed, and it's not even represented in a schema, no 2-way syncing between docs pages.

As I said in the motivations:

I think they (the attributes) should be done using a JSON object as representation schema, to automatically link attributes to their specific elements doc page, and list the specific elements doc page from the attribute section in the general input element doc page.
This is an attempt to make a manual update about them, albeit not being enough
skyclouds2001
skyclouds2001 reviewed Jan 11, 2024
View reviewed changes
Copy link
Contributor

@skyclouds2001 skyclouds2001 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that valueAsDate and valueAsNumber should not be listed in the Attributes section of the <input> element, they are not html element attribute after all
skyclouds2001
skyclouds2001 reviewed Jan 11, 2024
View reviewed changes
files/en-us/web/html/element/input/index.md
| [`src`](#src) | `image` | Same as `src` attribute for {{htmlelement('img')}}; address of image resource |
| [`step`](#step) | `date`, `month`, `week`, `time`, `datetime-local`, `number`, `range` | Incremental values that are valid |
| [`type`](#type) | all | Type of form control |
| [`value`](#value) | all except `image` | The value of the control. When specified in the HTML, corresponds to the initial value |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| [`value`](#value) | all except `image` | The value of the control. When specified in the HTML, corresponds to the initial value |
| [`value`](#value) | all except `image` | The value of the control. When specified in the HTML, corresponds to the initial value |

Personlly, the change to this sentence seems to be meanless
Copy link
Contributor Author

@Malix-off Malix-off Jan 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggestion is invalid because no changes were made to the code

The original value html input element description seems that "the value of the initial control" seems to be too limited, as value is simply the value of the control, and when specified in the HTML, it corresponds to the initial value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally it read The initial value of the control, which was accurate. this is about what the value attribute is when you are writing HTML, not the IDL attribute when you are querying the current value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this makes sense to me
@github-actions github-actions bot added the size/m [PR only] 51-500 LoC changed label Feb 29, 2024
@Malix-off Malix-off marked this pull request as ready for review June 22, 2024 10:37
@Malix-off Malix-off requested a review from a team as a code owner June 22, 2024 10:37
@Malix-off Malix-off requested review from dipikabh and removed request for a team June 22, 2024 10:37
Malix-off and others added 4 commits June 22, 2024 12:37
@Malix-off @estelle
fix: typo
48987ca 
Co-authored-by: Estelle Weyl <estelle@openwebdocs.org>
@estelle @Malix-off
fixed lint issues locally
cc1395c 
i fixed the bot issues locally. hopefully this fixes the bot issue.
@Malix-off
fix: indentation
98f6ab5
@Malix-off
fix: valueAsNumber and valueAsNumber should not be in files\en-us…
85d61bc 
…\web\html\element\input\index.md

#31639 (comment)
@Josh-Cena
Copy link
Member

Only the input element pages have "IDL attributes" in the technical summary. IMO we should just remove this row since all information can be found in the HTMLInputElement page anyway.
@dipikabh dipikabh removed their request for review July 11, 2024 17:04
@estelle
Copy link
Member

estelle commented Jul 11, 2024

Only the input element pages have "IDL attributes" in the technical summary. IMO we should just remove this row since all information can be found in the HTMLInputElement page anyway.

Let's do that in a different PR.
estelle
estelle approved these changes Jul 11, 2024
View reviewed changes
Copy link
Member

@estelle estelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.
Also, congratulations on your first merged MDN content PR.
Welcome to the team!
@estelle estelle merged commit 68ee1b1 into mdn:main Jul 11, 2024
8 checks passed
@Malix-off Malix-off deleted the patch-2 branch July 11, 2024 23:20
evelinabe pushed a commit to evelinabe/mdn-content that referenced this pull request Jul 12, 2024
@Malix-off @estelle @evelinabe
enhance: html input element - technical summary - attributes (mdn#31639)
93df74c 
* enhance: html input element - techincal summary - attributes

* fix hooks

* fix: typo

Co-authored-by: Estelle Weyl <estelle@openwebdocs.org>

* fixed lint issues locally

i fixed the bot issues locally. hopefully this fixes the bot issue.

* fix: indentation

* fix: `valueAsNumber` and `valueAsNumber` should not be in files\en-us\web\html\element\input\index.md

mdn#31639 (comment)

---------

Co-authored-by: Estelle Weyl <estelle@openwebdocs.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTML Hypertext Markup Language docs size/m [PR only] 51-500 LoC changed
4 participants
@Malix-off @estelle @Josh-Cena @skyclouds2001