Skip to content

New pages: DOMMatrixReadOnly.skewX/skewY/rotate #37152

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

Merged
merged 36 commits into from
Jan 30, 2025
Merged

New pages: DOMMatrixReadOnly.skewX/skewY/rotate #37152

merged 36 commits into from
Jan 30, 2025

Conversation

estelle
Copy link
Member

@estelle estelle commented Dec 9, 2024

@estelle estelle requested a review from a team as a code owner December 9, 2024 19:50
@estelle estelle requested review from wbamberg and removed request for a team December 9, 2024 19:50
@github-actions github-actions bot added Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed labels Dec 9, 2024
Copy link
Contributor

github-actions bot commented Dec 9, 2024

Preview URLs (9 pages)
Flaws (7)

Note! 4 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/API/DOMMatrixReadOnly/skewY
Title: DOMMatrixReadOnly: skewY() method
Flaw count: 1

  • macros:
    • Can't resolve /en-US/docs/Web/API/DOMMatrix/skewY

URL: /en-US/docs/Web/API/DOMMatrixReadOnly/skewX
Title: DOMMatrixReadOnly: skewX() method
Flaw count: 1

  • macros:
    • Can't resolve /en-US/docs/Web/API/DOMMatrix/skewX

URL: /en-US/docs/Web/API/DOMMatrix/rotateAxisAngleSelf
Title: DOMMatrix: rotateAxisAngleSelf() method
Flaw count: 2

  • macros:
    • Can't resolve /en-US/docs/Web/API/DOMMatrix/is2d
    • Can't resolve /en-US/docs/Web/API/DOMMatrix/is2d

URL: /en-US/docs/Web/API/DOMMatrix/rotateFromVectorSelf
Title: DOMMatrix: rotateFromVectorSelf() method
Flaw count: 2

  • macros:
    • Can't resolve /en-US/docs/Web/API/DOMMatrixRead/rotateSelf
    • Can't resolve /en-US/docs/Web/API/DOMMatrixRead/rotateAxisAngleSelf

URL: /en-US/docs/Web/API/DOMMatrix/scale3dSelf
Title: DOMMatrix: scale3dSelf() method
Flaw count: 1

  • macros:
    • Can't resolve /en-US/docs/Web/API/DOMMatrix/scaleSelf

(comment last updated: 2025-01-29 20:18:50)

@github-actions github-actions bot added size/l [PR only] 501-1000 LoC changed and removed size/m [PR only] 51-500 LoC changed labels Dec 10, 2024
Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

There were a few comment which applied to most of these pages, that I didn't note everywhere:

  • consider pretty-printing comments (and try to keep tgem under 80 characters, or they get hard-wrapped)
  • I'm not sure how relevant the CSS "see also" links are.

Comment on lines 45 to 47
// "matrix3d(0.728, 0.609, -0.315, 0, -0.525, 0.791, 0.315, 0, 0.441, -0.063, 0.895, 0, 0, 0, 0, 1)"
console.log(matrix.toString());
// "matrix3d(0.728, 0.609, -0.315, 0, -0.525, 0.791, 0.315, 0, 0.441, -0.063, 0.895, 0, 0, 0, 0, 1)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be worth pretty-printing these comments so they are more readable.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This still isn't addressed (and will apply to other pages too).

Comment on lines 63 to 77
- CSS {{cssxref("transform")}} property
- CSS {{cssxref("rotate")}} property
- CSS {{cssxref("transform-function")}} functions
- {{cssxref("transform-function/rotate", "rotate()")}}
- {{cssxref("transform-function/rotate3d", "rotate3d()")}}
- {{cssxref("transform-function/rotateX", "rotateX()")}}
- {{cssxref("transform-function/rotateY", "rotateY()")}}
- {{cssxref("transform-function/rotateZ", "rotateZ()")}}
- [CSS transforms](/en-US/docs/Web/CSS/CSS_transforms) module
- SVG [`transform`](/en-US/docs/Web/SVG/Attribute/transform) attribute
- {{domxref("CanvasRenderingContext2D")}} interface methods
- {{domxref("CanvasRenderingContext2D.rotate()")}}
- {{domxref("CanvasRenderingContext2D.transform()")}}
- {{domxref("CanvasRenderingContext2D.setTransform()")}}
- {{domxref("CanvasRenderingContext2D.resetTransform()")}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not that clear what the relevance of these transform/rotate pages are to this method. I mean it's not like you need to use matrices to rotate an item in CSS.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This also isn't addressed.

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 removed many of the links, but kept the links to CSS. It's way easier to rotate with CSS, so pointing them to a simpler way of doing in CSS what this is doing in JS.

@estelle estelle requested a review from wbamberg January 12, 2025 03:00
Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

Thanks! A few changes still are not addressed here:

#37152 (comment)
#37152 (comment)
#37152 (comment)
#37152 (comment)

estelle and others added 2 commits January 29, 2025 12:17
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@estelle estelle requested a review from wbamberg January 29, 2025 20:17
Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

👍

@wbamberg wbamberg merged commit e65acfe into main Jan 30, 2025
12 checks passed
@wbamberg wbamberg deleted the dmrotransforms branch January 30, 2025 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/l [PR only] 501-1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants