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

add: SVG Angle MDN Feature Pages #37013

Merged
merged 33 commits into from
Jan 7, 2025
Merged

Conversation

yashrajbharti
Copy link
Contributor

Description

This PR adds the MDN feature pages for the SVGAngle interface as part of a recent project to document missing Widely available pages for interface features.

Motivation

I believe that my support in documenting the SVGAngle interface can significantly help developers by providing essential resources.

Additional details

For more context, see the discussion on missing documentation tracked at https://openwebdocs.github.io/web-docs-backlog/baseline/. Additionally, related projects for adding widely available web features can be found at openwebdocs/project#214.

Related issues and pull requests

Relates to #214

@yashrajbharti yashrajbharti requested a review from a team as a code owner November 28, 2024 09:41
@yashrajbharti yashrajbharti requested review from wbamberg and removed request for a team November 28, 2024 09:41
@github-actions github-actions bot added Content:WebAPI Web API docs size/l [PR only] 501-1000 LoC changed labels Nov 28, 2024
Copy link
Contributor

github-actions bot commented Nov 28, 2024

Preview URLs (7 pages)
Flaws (2)

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

URL: /en-US/docs/Web/API/SVGAngle
Title: SVGAngle
Flaw count: 2

  • macros:
    • /en-US/docs/Web/API/SVGSVGElement/createSVGAngle does not exist
    • /en-US/docs/Web/API/SVGSVGElement/createSVGAngle does not exist

(comment last updated: 2025-01-07 16:53:23)

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 for your PR!

Please review the templates we use for reference pages: https://developer.mozilla.org/en-US/docs/MDN/Writing_guidelines/Page_structures/Page_types#api_reference_subpage and be sure to follow them. If you have any questions about specific aspects of the templates, please feel free to ask.

@yashrajbharti
Copy link
Contributor Author

Thanks for the resources, I will be correcting my way of writing the docs using this! and will fix the mistakes on this (and some other PRs soon)! I intend to write for clarity and completeness and this helps!

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

@wbamberg Thanks for referring me to the Page Templates. I believe most of the format is now correct. I do have a question: If the value of any property (e.g., valueInSpecifiedUnits) throws a DOMException, where is that generally documented?

@wbamberg
Copy link
Collaborator

wbamberg commented Dec 5, 2024

@wbamberg Thanks for referring me to the Page Templates. I believe most of the format is now correct. I do have a question: If the value of any property (e.g., valueInSpecifiedUnits) throws a DOMException, where is that generally documented?

That's a good question! I think the answer in general is: we don't document exceptions like this. For example, in the case you gave, setting valueInSpecifiedUnits throws if the object is read only, but that's going to be true of any property, so it's not helpful to mention it.

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 for the updates. I've left comments on just one of the pages, as I expect similar comments are going to apply to the others.

Apart from things like formatting conventions, the tricky part is thinking about: what do we want to tell readers about these APIs? What do they need to know about them? Just paraphrasing the spec is usually not helpful, because then they might as well go and read the spec.

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 for the updates!

I started reviewing this but stopped when I got to https://github.com/mdn/content/pull/37013/files#diff-9122079f4f6ce53752a712afdd280a97e8be60040e0739ccb94c1f96bb2a492e, because it seems like it doesn't followed the template either.

Please, check that all the pages follow the template, and that any other comments that seem to be applicable to other pages (for example, the way the unit types are described and formatted) are followed. Otherwise I have to make the exact same comment again and again, and it isn't very efficient.

@wbamberg
Copy link
Collaborator

wbamberg commented Jan 6, 2025

@wbamberg In the UnitType Page, I was unsure if we will document the unique constants that each of the SVGAngle.SVG_ANGLETYPE corresponds to. (For example SVGAngle.SVG_ANGLETYPE_DEG = 2)

I think actually since we have https://developer.mozilla.org/en-US/docs/Web/API/SVGAngle#constants, then we should not document the constant values in these pages at all, and just link to https://developer.mozilla.org/en-US/docs/Web/API/SVGAngle#constants. I'm not sure whether we should document the numeric values (but if we do, it should be in https://developer.mozilla.org/en-US/docs/Web/API/SVGAngle#constants).

@yashrajbharti
Copy link
Contributor Author

@wbamberg Thanks for your answer, when I was documenting, I saw constants documented in the parent's interface in almost all cases. Such as SVGTransform | Constants, SVGTextContentElement | Constants, SVGFEColorMatrixElement | Constants the list goes on.

However it also seems these are following an older format as data is represented in tables.

With that said, only SVGAngle seems to miss out on constants being documented.

files/en-us/web/api/svgangle/unittype/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/svgangle/unittype/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/svgangle/valueasstring/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/svgangle/value/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/svgangle/unittype/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/svgangle/valueasstring/index.md Outdated Show resolved Hide resolved
Co-authored-by: wbamberg <will@bootbonnet.ca>
yashrajbharti and others added 9 commits January 7, 2025 14:46
Co-authored-by: wbamberg <will@bootbonnet.ca>
Co-authored-by: wbamberg <will@bootbonnet.ca>
Co-authored-by: wbamberg <will@bootbonnet.ca>
Co-authored-by: wbamberg <will@bootbonnet.ca>
Co-authored-by: wbamberg <will@bootbonnet.ca>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: wbamberg <will@bootbonnet.ca>
@yashrajbharti yashrajbharti requested a review from wbamberg January 7, 2025 09:23
@yashrajbharti
Copy link
Contributor Author

Made the fixes in value, and removed the part where we assumed angle as instance when we createSVGAngle as it will always be the case, so the word assume isn't required.

Copy link
Contributor

github-actions bot commented Jan 7, 2025

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot added the merge conflicts 🚧 [PR only] label Jan 7, 2025
@github-actions github-actions bot removed the merge conflicts 🚧 [PR only] label Jan 7, 2025
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.

👍 looks great, thank you @yashrajbharti !

@wbamberg wbamberg merged commit a5de116 into mdn:main Jan 7, 2025
8 checks passed
@yashrajbharti yashrajbharti deleted the feat/SVGAngle branch January 7, 2025 17:43
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/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants