-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
docs(src/webgl): Use describe()
instead of @alt
#5599
Conversation
@lm-n ping 😇 |
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.
Great job! Can you help edit the text of long descriptions to make them more succinct? It'd be great if none of the descriptions are more than 3 lines long.
src/webgl/3d_primitives.js
Outdated
* describe(`A white rotating sphere-like shape with a diameter of 40. | ||
* Its actual shape is similar to a clam shell. | ||
* Black lines on its surface show how the sphere is built out of | ||
* many small triangles. | ||
* When adjusting the slider below the drawing, the shape increases | ||
* the number of small triangles making up the sphere, making the | ||
* final shape increasingly round as the slider approaches its | ||
* maximum setting.`); |
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.
This is a very long description. Can we try to make it more succinct?
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.
As I tried to fulfill this request, I was able to rewrite this into something that I think is an improvement in quality, but isn’t much shorter.
The challenges for a short description in this demo (and other similar demos) are:
- The description of the main shape isn’t the whole story. The entire point of this (and similar) demo(s) is to show how a big shape is built out of smaller shapes. Thus, for the description to include all the relevant info, it needs to include both the overall shape as well as the smaller triangles that comprise it.
- The description also needs to describe the slider (outside of the drawing), and how that slider’s value affects the drawing itself.
These aren’t conditions that most of the other demos have. But since this demo is all about how the level of detail impacts both the visual and the performance, it seems to me that leaving that information out is omitting crucial information.
Do you think this is sufficient to justify a slightly longer description?
(To be clear: I still think I can come up with a rewrite that is an improvement—just not substantially shorter.)
* describe(`A white rotating sphere-like shape with a diameter of 40. | ||
* Its actual shape is like a cylinder with the top and bottom | ||
* surface protruding outward. | ||
* Black lines on its surface show how the sphere is built out of | ||
* many small triangles. | ||
* When adjusting the slider below the drawing, the shape increases | ||
* the number of small triangles making up the sphere, making the | ||
* final shape increasingly round as the slider approaches its | ||
* maximum setting.`); |
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.
Same as above, can we try to make it more succinct?
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.
Same response as my first comment.
* describe(`A spinning view of what appears to be a white plane. | ||
* Black lines on its surface show how the shape is built out of | ||
* many small triangles. | ||
* As you increase the slider below the canvas, the shape gradually | ||
* becomes smoother and closer to a cylinder.`); |
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.
Can we make description more succinct?
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.
Same response as my first comment.
* describe(`A spinning view of a white cylinder. | ||
* Black lines on its surface show how the shape is built out of | ||
* many small triangles. | ||
* As you increase the slider below the canvas, the shape increases | ||
* the number of triangles on the outer (round) surface, but the | ||
* smoothness of the shape remains the same.`); |
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.
Can we make description more succinct? For instance: a spinning white cylinder with its surface made of small triangles, the slider below the canvas modifies the number of triangles on the shape.
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.
Same response as my first comment.
* describe(`a 3D box is centered on a grid in a 3D sketch. | ||
* an icon indicates the direction of each axis: | ||
* a red line points +X, a green line +Y, and a blue line +Z. | ||
* the grid and icon disappear when the spacebar is pressed.`); |
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.
can we make more succinct?
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.
Same response as my first comment.
* describe(`a 3D box is centered on a grid in a 3D sketch. | ||
* an icon indicates the direction of each axis: | ||
* a red line points +X, a green line +Y, and a blue line +Z. | ||
* the grid and icon disappear when the spacebar is pressed.`); |
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.
can we make it more succinct?
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.
Same response as my first comment.
* describe(`Canvas toggles between a circular gradient of | ||
* orange and blue vertically. And a circular gradient of | ||
* red and green moving horizontally when mouse is | ||
* clicked/pressed.`); |
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.
can we make more succinct?
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.
Same response as my first comment.
69135f1
to
12702f8
Compare
Addresses #5139. Attention @lm-n. ;-)
Changes:
Use
describe()
over@alt
in inline docs within 📁 src/webgl/.Several descriptions for
@alt
(especially in3d_primitives.js
) were also incorrect. I wrote new descriptions for these examples based on what I saw in the browser.PR Checklist
npm run lint
passes