-
Notifications
You must be signed in to change notification settings - Fork 199
Add transform-box, add missing transform properties #1695
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
Conversation
autonome
left a comment
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.
Pre-review, a couple of description suggestions.
features/individual-transforms.yml
Outdated
| @@ -1,4 +1,11 @@ | |||
| name: Individual transform properties | |||
| description: Transform elements with separate `translate`, `rotate`, and `scale` CSS properties. | |||
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.
non-blocking suggestion: The description could be more explicit about how these allow individual transformation vs the grouped property, giving context to why the feature is named as such. Eg, I like the "simple transforms independently" in the spec language.
features/transform-box.yml
Outdated
| @@ -0,0 +1,10 @@ | |||
| name: transform-box | |||
| description: The `transform-box` CSS property sets the reference box for transformations. | |||
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.
Ditto, I like the spec language, which gives just a bit more colour: "relative to the position and dimensions of the reference box". (The MDN description I found oddly phrased.)
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.
"Oddly phrased" seems to be the standard when looking at descriptions of this property. I updated it, and hope this is one standard I didn't match, but the phrasing is tricky.
captainbrosset
left a comment
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.
Approving with a comment on the description. But it feels good enough for me not to have to block/review again.
features/transform-box.yml
Outdated
| @@ -0,0 +1,10 @@ | |||
| name: transform-box | |||
| description: The `transform-box` CSS property sets the position and dimensions, or reference box, relative to which transformations are calculated. | |||
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.
| description: The `transform-box` CSS property sets the position and dimensions, or reference box, relative to which transformations are calculated. | |
| description: The `transform-box` CSS property sets the position and dimensions of the reference box relative to which an element's individual transformations are calculated. |
I was having trouble parsing the initial sentence. Maybe I misunderstood what you meant.
I added the word "individual" in there in an attempt to make it clear that this only applies for individual transform properties (if, indeed, this is how this works).
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 liked the phrasing, but removed the word individual- it applies to the individual transform properties (scale, translate, rotate) as well as transform.
ddbeck
left a comment
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.
Happy enough to merge. Thank you!
No description provided.