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

Add prose and meta-docs for [LegacyNullToEmptyString] #28213

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

teoli2003
Copy link
Member

Description

This PR adds relevant information to existing MDN pages about properties or method parameters prefixed with the WebIDL [LegacyNullToEmptyString].

Motivation

This is part of openwebdocs/project#159 where we want to uniformize and add more information stored in WebIDLs files. Standardizing them makes them easier to understand and MDN Web docs articles more precise.

Additional details

What does this extended attribute mean?

This extended attribute alters how null values are converted to a string. The standard way is to convert null to the string "null". When this extended attribute is present, the null object is converted to an empty string ("").

What does this PR do?

  • It adds to all relevant properties a sentence explaining the set behaviour when set to the null value, with a tiny inline example.
  • It adds to all such parameters a sentence explaining what the behaviour is when null is passed.
  • Update the meta-docs, listing the standardized sentences.

Related issues and pull requests

None.

@teoli2003 teoli2003 marked this pull request as ready for review July 27, 2023 10:19
@teoli2003 teoli2003 requested review from a team as code owners July 27, 2023 10:19

Add the following sentence to the end of the _Value_ section of the article:

_When set to the `null` value, that `null` value is converted to the empty string (`""`), unlike the common behavior with strings (that would have converted it to the `"null"` string): `elt.innerHTML = null` is equivalent to `elt.innerHTML = ""`._
Copy link
Collaborator

@wbamberg wbamberg Jul 27, 2023

Choose a reason for hiding this comment

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

I think it would be worth simplifying and shortening this:

Suggested change
_When set to the `null` value, that `null` value is converted to the empty string (`""`), unlike the common behavior with strings (that would have converted it to the `"null"` string): `elt.innerHTML = null` is equivalent to `elt.innerHTML = ""`._
_When set to the `null` value, that `null` value is converted to the empty string (`""`), so `elt.innerHTML = null` is equivalent to `elt.innerHTML = ""`._

So I'm not sure it's worth describing what doesn't happen, just what does happen.

Also I think it's worth adding this meta-guidance (or at least a link to it) to our page templates, since that's the main place people go to figure out how to write reference pages.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a link in the template.

@Rumyra
Copy link
Collaborator

Rumyra commented Aug 2, 2023

All looks good to me! Agree with @wbamberg suggestions and one nit :)

@wbamberg
Copy link
Collaborator

@teoli2003 , are you coming back to this, or should we close it for now?

@bsmth bsmth added the awaiting response Awaiting for author to address review/feedback label Jan 10, 2024
@bsmth
Copy link
Member

bsmth commented Jan 10, 2024

Hi @teoli2003 shall we close this for now?

files/en-us/web/api/element/innerhtml/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/window/open/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/window/open/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/window/open/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/window/open/index.md Outdated Show resolved Hide resolved
teoli2003 and others added 10 commits January 10, 2024 13:25
…e/information_contained_in_a_webidl_file/index.md

Co-authored-by: Ruth John <Rumyra@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
teoli2003 and others added 6 commits January 10, 2024 13:25
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…e/information_contained_in_a_webidl_file/index.md
@teoli2003
Copy link
Member Author

Hi @teoli2003 shall we close this for now?

I have fixed all open points and rebased. Let's ask for a re-review.

@teoli2003 teoli2003 removed the awaiting response Awaiting for author to address review/feedback label Jan 10, 2024
@@ -25,7 +25,7 @@ setProperty(propertyName, value, priority)
- : A string representing the CSS property name (hyphen case) to be modified.
- `value` {{optional_inline}}
- : A string containing the new property value. If not specified, treated
as the empty string.
as the empty string. A ["null"](/en-US/docs/Web/JavaScript/Reference/Operators/null) value is treated the same as the empty string (`""`).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
as the empty string. A ["null"](/en-US/docs/Web/JavaScript/Reference/Operators/null) value is treated the same as the empty string (`""`).
as the empty string. A [`null`](/en-US/docs/Web/JavaScript/Reference/Operators/null) value is treated the same as the empty string (`""`).

Also, shouldn't this be merged with the previous sentence? "If null or unspecified, it is treated the same as the empty string ("")"

Parameter types may have special behaviors described using extended attributes (like `[LegacyNullToEmptyString]`). Here is the list of such attributes, and the addition you have to do in the prose:

- `[LegacyNullToEmptyString]`
- : Add the following sentence at the end of the parameter description: _A ["null"](/en-US/docs/Web/JavaScript/Reference/Operators/null) value is treated the same as the empty string (`""`)._
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- : Add the following sentence at the end of the parameter description: _A ["null"](/en-US/docs/Web/JavaScript/Reference/Operators/null) value is treated the same as the empty string (`""`)._
- : Add the following sentence at the end of the parameter description: _A [`null`](/en-US/docs/Web/JavaScript/Reference/Operators/null) value is treated the same as the empty string (`""`)._

- : A string containing the qualified name, that is an optional
prefix and colon plus the local root element name, of the document to be created.
prefix and colon plus the local root element name, of the document to be created. A ["null"](/en-US/docs/Web/JavaScript/Reference/Operators/null) value is treated the same as the empty string (`""`).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
prefix and colon plus the local root element name, of the document to be created. A ["null"](/en-US/docs/Web/JavaScript/Reference/Operators/null) value is treated the same as the empty string (`""`).
prefix and colon plus the local root element name, of the document to be created. A [`null`](/en-US/docs/Web/JavaScript/Reference/Operators/null) value is treated the same as the empty string (`""`).

files/en-us/web/api/window/open/index.md Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented May 2, 2024

This pull request has merge conflicts that must be resolved before it can be merged.

Co-authored-by: Joshua Chen <sidachen2003@gmail.com>
@github-actions github-actions bot added the size/m 51-500 LoC changed label May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Meta Content in the meta docs Content:WebAPI Web API docs merge conflicts 🚧 size/m 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants