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

style: use title case for attr-translate-yes-state and attr-translate… #11027

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

Conversation

HarshitGourlariya
Copy link

@HarshitGourlariya HarshitGourlariya commented Feb 14, 2025

…-no-state
Use title case for attr-translate-* attributes

This PR corrects the capitalization of several attr-translate-* attributes to use title case. This ensures consistency in how translation-related attributes are handled and improves the overall readability of the HTML specification.

Specifically, the following attributes were updated:

  • attr-translate-yes-state
  • attr-translate-no-state
  • At least two implementers are interested (and none opposed):
  • Tests are written and can be reviewed and commented upon at:
  • Implementation bugs are filed:
    • Chromium: …
    • Gecko: …
    • WebKit: …
    • Deno (only for timers, structured clone, base64 utils, channel messaging, module resolution, web workers, and web storage): …
    • Node.js (only for timers, structured clone, base64 utils, channel messaging, and module resolution): …
  • Corresponding HTML AAM & ARIA in HTML issues & PRs:
  • MDN issue is filed: …
  • The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)


/dnd.html ( diff )
/dom.html ( diff )
/embedded-content.html ( diff )
/form-control-infrastructure.html ( diff )
/forms.html ( diff )
/images.html ( diff )
/infrastructure.html ( diff )
/interaction.html ( diff )
/parsing.html ( diff )
/popover.html ( diff )
/scripting.html ( diff )
/semantics-other.html ( diff )
/semantics.html ( diff )
/tables.html ( diff )
/urls-and-fetching.html ( diff )

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

You need to remove the grep_results.txt file. I also have a number of nits. Note that "Title Case" means that each word has to start with a capital.

@@ -1857,7 +1857,7 @@ a.setAttribute('href', 'https://example.com/'); // change the content attribute
</li>

<li><p>If <var>removedNode</var>'s <code data-x="attr-popover">popover</code> attribute is not in
the <span data-x="attr-popover-none-state">no popover state</span>, then run the <span>hide
the <span data-x="attr-popover-none-state">No popover state</span>, then run the <span>hide
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be "No Popover State", right?

Copy link
Author

Choose a reason for hiding this comment

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

ok Working on it .

Copy link
Member

Choose a reason for hiding this comment

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

This is still not addressed.

Copy link
Author

Choose a reason for hiding this comment

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

"I've resolved the issue. Please review."

@@ -13770,22 +13770,22 @@ Transport Protocol">HTTP&lt;/abbr> today.&lt;/p></code></pre> <!-- DO NOT REWRAP
<tbody>
<tr>
<td><dfn attr-value for="html-global/dir"><code data-x="attr-dir-ltr">ltr</code></dfn>
<td><dfn data-x="attr-dir-ltr-state">ltr</dfn>
<td><dfn data-x="attr-dir-ltr-state">Ltr</dfn>
Copy link
Member

Choose a reason for hiding this comment

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

This should be LTR and Rtl should be RTL.

<td>The header cell applies to some of the subsequent cells in the same column(s).
<tr>
<td><dfn attr-value for="th/scope"><code data-x="attr-th-scope-rowgroup">rowgroup</code></dfn>
<td><dfn data-x="attr-th-scope-rowgroup-state">row group</dfn>
<td><dfn data-x="attr-th-scope-rowgroup-state">Row group</dfn>
Copy link
Member

Choose a reason for hiding this comment

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

Row Group, also Column Group below.

have to take care when writing their style sheets to make sure that the attribute is still styled
as expected. In addition, legacy user agents which don't support the <span
data-x="attr-hidden-until-found-state">hidden until found</span> state will have 'display: none'
data-x="attr-hidden-until-found-state">Hidden until found</span> state will have 'display: none'
Copy link
Member

Choose a reason for hiding this comment

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

This would be Hidden Until Found.

@@ -30431,7 +30431,7 @@ was an English &lt;a href="/wiki/Music_hall">music hall&lt;/a> singer, ...</code
<div class="example">

<p>This example is the same as the previous example, but the image is <span
data-x="attr-loading-lazy-state">lazy-loaded</span>. In this case, the <code
data-x="attr-loading-lazy-state">Lazy-loaded</span>. In this case, the <code
Copy link
Member

Choose a reason for hiding this comment

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

Lazy-Loaded or maybe Lazy Loaded.

source Outdated
@@ -82941,20 +82942,20 @@ body { display:none }
data-x="">window.find()</code> API.</p>

<h4>Interaction with <code>details</code> and <code
data-x="attr-hidden-until-found-state">hidden=until-found</code></h4>
data-x="attr-hidden-until-found-state">Hidden=until-found</code></h4>
Copy link
Member

Choose a reason for hiding this comment

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

Here the casing should probably not change.

@HarshitGourlariya
Copy link
Author

"I've addressed the issue identified in the review. The calculation error has been corrected. Please take another look."

@annevk
Copy link
Member

annevk commented Feb 14, 2025

It still says that this PR changes 2 files so I think you need to take another look.

@HarshitGourlariya
Copy link
Author

I've removed grep_results.txt from the PR as it was mistakenly included.
The commit now accurately reflects the intended changes.

@@ -1857,7 +1857,7 @@ a.setAttribute('href', 'https://example.com/'); // change the content attribute
</li>

<li><p>If <var>removedNode</var>'s <code data-x="attr-popover">popover</code> attribute is not in
the <span data-x="attr-popover-none-state">no popover state</span>, then run the <span>hide
the <span data-x="attr-popover-none-state">No popover state</span>, then run the <span>hide
Copy link
Member

Choose a reason for hiding this comment

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

This is still not addressed.

@@ -75065,7 +75065,7 @@ Demos:
<p>The <code data-x="selector-popover-open">:popover-open</code> <span>pseudo-class</span> is
defined to match any <span data-x="html elements">HTML element</span> whose <code
data-x="attr-popover">popover</code> attribute is not in the <span
data-x="attr-popover-none-state">no popover state</span> and whose <span>popover visibility
data-x="attr-popover-none-state">No popover state</span> and whose <span>popover visibility
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

<td rowspan=2>Will not be rendered.
<tr>
<td>(the empty string)
<tr>
<td><dfn attr-value for="html-global/hidden"><code data-x="attr-hidden-until-found">until-found</code></dfn>
<td><dfn data-x="attr-hidden-until-found-state">hidden until found</dfn>
<td><dfn data-x="attr-hidden-until-found-state">Hidden until found</dfn>
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

<td>Will not be rendered, but content inside will be accessible to <span>find-in-page</span>
and <span data-x="navigate-fragid">fragment navigation</span>.
</table>

<p>The attribute's <i data-x="missing value default">missing value default</i> is the <dfn
data-x="attr-hidden-not-hidden-state">not hidden</dfn> state, and its <i data-x="invalid value
default">invalid value default</i> is the <span data-x="attr-hidden-hidden-state">hidden</span>
data-x="attr-hidden-not-hidden-state">Not hidden</dfn> state, and its <i data-x="invalid value
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

indirectly through the style layer. For example, a web browser could implement these requirements
<a href="#hiddenCSS">using the rules suggested in the Rendering section</a>.</p>

<p>When an element has the <code data-x="attr-hidden">hidden</code> attribute in the <span
data-x="attr-hidden-until-found-state">hidden until found</span> state, it indicates that the
element is hidden like the <span data-x="attr-hidden-hidden-state">hidden</span> state but the
data-x="attr-hidden-until-found-state">Hidden until found</span> state, it indicates that the
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

@@ -79018,35 +79018,35 @@ END:VCARD</pre>

<p>Web browsers will use 'content-visibility: hidden' instead of 'display: none' when the <code
data-x="attr-hidden">hidden</code> attribute is in the <span
data-x="attr-hidden-until-found-state">hidden until found</span> state, as specified in the <a
data-x="attr-hidden-until-found-state">Hidden until found</span> state, as specified in the <a
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

I stopped here. Please make sure the entire PR uses title case for enumeration states. Thanks!

@HarshitGourlariya
Copy link
Author

ok

@HarshitGourlariya
Copy link
Author

"I've resolved the issue. Please review."

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Nice, I found one more to address (multiple times, to be clear), but looks like a solid improvement otherwise.

<td>The element is not editable.
<tr>
<td><dfn attr-value for="html-global/contenteditable"><code
data-x="attr-contenteditable-plaintextonly">plaintext-only</code></dfn>
<td><dfn data-x="attr-contenteditable-plaintextonly-state">plaintext-only</dfn>
<td><dfn data-x="attr-contenteditable-plaintextonly-state">Plaintext-only</dfn>
Copy link
Member

Choose a reason for hiding this comment

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

Has to be Plaintext-Only throughout.

@annevk
Copy link
Member

annevk commented Feb 17, 2025

@HarshitGourlariya apart from that, could you please read and sign https://participate.whatwg.org/agreement? Thank you!

@HarshitGourlariya
Copy link
Author

Thanks for reviewing my commits. I have made the changes related to the Plaintext-Only issue.

@annevk
Copy link
Member

annevk commented Feb 17, 2025

It seems you only fixed one instance?

@HarshitGourlariya
Copy link
Author

You said you found "Plaintext-Only" throughout the entire code, which was not properly addressed. So, I made changes to that in my latest commit.

@annevk
Copy link
Member

annevk commented Feb 18, 2025

I said it has to be "Plaintext-Only" throughout. You changed one instance of "Plaintext-only" to "Plaintext-Only", but my comment signified (through the use of "throughout") that there are multiple instances of "Plaintext-only" that need to be replaced.

@HarshitGourlariya
Copy link
Author

"I found only two plaintext-related issues and have updated them. Please review."

@annevk
Copy link
Member

annevk commented Feb 19, 2025

That last commit went wrong it seems. I recommend double checking before pushing next time.

@HarshitGourlariya
Copy link
Author

I have fixed the issue. Please review.

@annevk
Copy link
Member

annevk commented Feb 19, 2025

@HarshitGourlariya

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants