-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Documentation: Document how to add a new CSS property #3564
base: master
Are you sure you want to change the base?
Conversation
Documentation/CSSProperties.md
Outdated
inherited or not, it needs adding to the `m_inherited` or `m_noninherited` structs, with a corresponding getter. | ||
- `MutableComputedValues` also needs a setter for the value. | ||
- `InitialValues` has a getter for the default value of the property. This isn't always needed if the default would be | ||
an empty `Vector` or similar. |
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.
"empty Vector
or similar" isn't super clear to me.
Does this just refer to whatever would be the default value for the given type anyways?
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've rewritten that sentence, is it clearer now?
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.
Yea, I think it is.
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.
Looks great, thanks for doing this! This document would have helped me significantly when I worked on font-variant and friends. Adding a few more notes on things that I ran into.
might need to modify `ShorthandStyleValue::to_string` if your shorthand has special serialization rules. | ||
|
||
If your property's value can't be represented with an existing type, you might need to add a new style value class. | ||
If you need to do this, pester @AtkinsSJ until he gets around to documenting it. ;^) |
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 something about custom serialization rules here?
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'm not 100% sure what you mean, but I've added an example of a custom serialization rule, does that work?
Documentation/CSSProperties.md
Outdated
|
||
Some properties have special rules for getting the computed value from JS. For these, you will need to add to | ||
`ResolvedCSSStyleDeclaration::style_value_for_property()`. Shorthands that are constructed in an unusual way may also | ||
need handling inside `CSSStyleDeclaration::get_property_internal()`. |
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.
Finally, when you have a resolved one, you should be able to use them, for example in the graphics stack by calling XXX.
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 added an example to the section above. The part you highlighted here is meant to be about JS access so I've changed the subheading.
This should probably be linked from Documentation/README.md#browserlibweb |
5cccafd
to
0638543
Compare
Thanks for the feedback! I think I've addressed it but I'm not entirely sure what some comments referred to, so let me know if I didn't. |
I've been meaning to write this for ages. Hopefully it makes sense! Feedback welcome.