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

Guidance about reflecting state in HTML attributes #501

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

Conversation

LeaVerou
Copy link
Member

@LeaVerou LeaVerou commented Jul 15, 2024

@LeaVerou LeaVerou requested review from hober and martinthomson July 15, 2024 18:54
Copy link
Contributor

@hober hober left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@@ -1032,6 +1032,25 @@ The alternative user experience is a flash of unstyled content, which is undesir

See also [[#worker-only]].

<h3 id="html-attribute-property-reflection">Expose content attributes for writable IDL attributes</h3>

When designing a new *writable* IDL attribute to specify element state,
Copy link
Contributor

Choose a reason for hiding this comment

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

The emphasis on "writable" doesn't seem to serve a purpose. (I, for one, find it distracting.)

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 disagree, I think "writable" is essential to the principle and should be emphasized.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest a rephrase then.

When designing a new IDL attribute that reflects the state of an element,
expose a corresponding content attribute,
unless the attribute is immutable or
there are other strong reasons not to.

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
- It takes advantage of HTML reactivity, and alleviates the need to apply changes at a specific moment in time.
- It integrates with existing DOM methods, improving learnability, whereas an API specific to the feature needs to be learned separately.
- Behaviors can be expressed reactively in libraries supporting HTML-based syntax and be handled by any library handling HTML generically.
- UI states can be addressed in CSS via simple attribute selectors rather than new pseudo-classes.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this is a good thing in all cases. For instance, [lang=foo] v. [lang|=foo] v. :lang(foo).

Copy link
Member Author

Choose a reason for hiding this comment

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

This seems orthogonal. In that case the pseudo-class serves a distinct purpose by targeting the actual computed language (which may be specified on an ancestor). I’d argue there may even be value in a generic attribute selector for these inherited attributes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd favor Lea's point on this one. :lang() is justified because it is not as much about element state directly and more about the state of the DOM tree, as viewed from that node.

That said, the other point is also good: [lang=foo] is possible, but inadvisable.

<{input}>'s {{HTMLInputElement/indeterminate}} IDL property
which was never exposed as a content attribute making it impossible to fully update checkbox state
by modifying HTML.
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

On the other hand, indeterminate is a terrible feature that should never have been added to the platform in the first place. Maybe we should try to find a more positive example?

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you elaborate? Is it the UI of an indeterminate checkbox that you think is a mistake or the way it was added to HTML?

Copy link
Contributor

Choose a reason for hiding this comment

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

All of the above, IIRC.

Copy link
Member

@annevk annevk Jul 17, 2024

Choose a reason for hiding this comment

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

The indeterminate IDL attribute is a bad example as it's like the checked or value IDL attributes which also do not have corresponding content attributes.

You could argue that we should have had a defaultIndeterminate IDL attribute for a corresponding indeterminate content attribute, but that's a separate matter. whatwg/html#6578 goes into this a bit.

(And of course people always like to point out that the IDL attributes not matching the names of the content attributes is confusing, but that's also a separate matter and anyway not something that can actually be solved.)

Copy link
Member Author

@LeaVerou LeaVerou Jul 17, 2024

Choose a reason for hiding this comment

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

There’s a separate, existing, principle about this: Keep attributes in sync. It’s right below this one actually 😁

indeterminate in addition to that, also has no content attribute counterpart so there is no HTML way to specify it at all (current OR initial status).

Copy link
Member Author

Choose a reason for hiding this comment

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

@hober Interesting. Could you elaborate on the UI part?

Copy link
Member

@annevk annevk Aug 21, 2024

Choose a reason for hiding this comment

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

That principle is about content attributes needing to be reflected right? But indeterminate is not a content attribute. Edit: ah right, it even says so in the note:

This does not hold the other way around. A new IDL attribute does not always warrant a content attribute counterpart.

Co-authored-by: Theresa O'Connor <[email protected]>
index.bs Outdated Show resolved Hide resolved
<{input}>'s {{HTMLInputElement/indeterminate}} IDL property
which was never exposed as a content attribute making it impossible to fully update checkbox state
by modifying HTML.
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

All of the above, IIRC.

Copy link
Contributor

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

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

I made this comment a VERY long time ago. I have not re-examined it for whether I still believe it.

- It takes advantage of HTML reactivity, and alleviates the need to apply changes at a specific moment in time.
- It integrates with existing DOM methods, improving learnability, whereas an API specific to the feature needs to be learned separately.
- Behaviors can be expressed reactively in libraries supporting HTML-based syntax and be handled by any library handling HTML generically.
- UI states can be addressed in CSS via simple attribute selectors rather than new pseudo-classes.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd favor Lea's point on this one. :lang() is justified because it is not as much about element state directly and more about the state of the DOM tree, as viewed from that node.

That said, the other point is also good: [lang=foo] is possible, but inadvisable.

@martinthomson martinthomson changed the title First stab at #289 Guidance about reflecting state in HTML attributes Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants