-
Notifications
You must be signed in to change notification settings - Fork 298
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
Refactor toggle attribute #663
base: main
Are you sure you want to change the base?
Refactor toggle attribute #663
Conversation
8c62e75
to
543336c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I quite like the direction this is going in. We do end up with a few more lines, but it's much more clear what is going on.
@@ -6189,6 +6189,40 @@ in an <a for="/">element</a> <var>element</var>, run these steps: | |||
|
|||
<hr> | |||
|
|||
|
|||
To <dfn export id=concept-element-attributes-set-by-name>create an attribute by value</dfn> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should name this "create and append an attribute"? Or maybe I don't understand "by value".
The other thing I'm thinking is that it would be nice if https://dom.spec.whatwg.org/#concept-element-attributes-set-value could invoke this algorithm as well. That would require additional arguments for namespace/prefix, but seems doable.
<a for=Node>node document</a>. | ||
|
||
<li><a lt="append an attribute">Append</a> this <a>attribute</a> to | ||
<a>element</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You want <var>element</var>
here and a dot.
<ol> | ||
<li><p>Create an <a>attribute</a> whose | ||
<a for="Attr">local name</a> is <var>qualifiedName</var>, <a for=Attr>value</a> is | ||
<var>value</var>, and <a for=Node>node document</a> is <a>context object</a>'s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the context object* throughout
<li>Return. | ||
</ol> | ||
|
||
To <dfn export id=concept-element-attributes-get-valid-by-name>get a valid attribute by name</dfn> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"validate and get an attribute by name" perhaps?
<li><p><a lt="remove an attribute by name">Remove an attribute</a> given <var>qualifiedName</var> and the | ||
<a>context object</a>. | ||
|
||
<li>Return false. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<li><p>
(also above)
Solves:
#657
whatwg/html#3779
As I understand it vars are not mutable and are instead copied, right? This mean I still have to copy from
"get an attribute by name"
the code.Would there be a way to simplify this further perhaps? I could copy out the two common steps in the body of
"get an attribute by name"
and"get valid an attribute by name"
into another concept?Are there better names for the concept?
💥 Error: 500 Internal Server Error 💥
PR Preview failed to build. (Last tried on Jan 15, 2021, 7:33 AM UTC).
More
PR Preview relies on a number of web services to run. There seems to be an issue with the following one:
🚨 CSS Spec Preprocessor - CSS Spec Preprocessor is the web service used to build Bikeshed specs.
🔗 Related URL
If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.