-
-
Notifications
You must be signed in to change notification settings - Fork 481
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
How to label your p5 canvas tutorial revisions #1435
How to label your p5 canvas tutorial revisions #1435
Conversation
….com/katlich112358/p5.js-website-gsoc-2023-v2 into how-to-label-your-p5-canvas-tutorial
@katlich112358 your revision looks solid! I left a few comments/suggestions. Thanks for taking the new writing guide for a spin–excited to incorporate your feedback. |
@nickmcintyre awesome, thank you! I think the new writing guide is pretty sound, my main struggle was with getting the grammar down to a 6th grade reading level (and even then I think it only made it to 9th), so maybe it could be useful to change the "grade level cap" to 8th or 9th? It more had to do with accurate code jargon naturally being above 6th grade level, I believe. 6th grade can be a good goal for grammar, but 8th is passable? The how-to step process worked well, and I really like the idea of the "Prerequisite" section! |
@katlich112358 good distinction between grammar and vocabulary–hadn't thought of that. I raised the "grade level cap" to 8th and will bring this up with other writers. Some things are just technical, and you struck a good balance in your how-to. Let me know how you'd like to proceed with the suggestions and we can request a merge. |
@nickmcintyre Thanks! I'm still getting used to GitHub and the review process, where can I find these reviews? I don't see any pending review check on my end. Where might I find them elsewhere? |
labeling-canvases-steps-for-labeling-step-2-using-de-3: >- | ||
If your canvas contains any <a class = "code" href = "https://p5js.org/reference/#/p5/text">text()</a> elements that are important to the general understanding of the | ||
image, make a separate label for them. Label any legible text with quotation marks around it in the | ||
label, as in <span class = "code">describeElement("Text", "The words 'hello, world' displayed in green at the center of a |
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.
Minor point, but the code style guide suggests using single quotes ''
and template strings `hello, ${name}`
.
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.
@nickmcintyre whoops forgot to explain that, the code widget freaked when I used single quotes and the ${} template strings? Is there something I can do within the code itself to make these styles work?
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.
@katlich112358 Those are considered special characters in YAML and need to be escaped. Escaping special characters in YAML is kinda messy though but there's not much way to get around 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.
@limzykenneth Fixed this issue, typo on my end. Thanks!
@katlich112358 oops, turns out I forgot to hit the submit button. |
Really excellent work on this @katlich112358! This is such a helpful resource. |
Do let us know if this is ready for merge. There are some merge conflicts, I can help resolve them if necessary. Thanks. |
@limzykenneth This is ready to merge, and I would appreciate some help in fixing the merge conflicts? Thank you! |
@katlich112358 I have one more suggestion based on this line:
I think it's actually ok to call |
@nickmcintyre Thanks for catching that! Just updated the copy. |
@nickmcintyre @limzykenneth Just added the last of the proofreading revisions, this PR is ready to merge. I'm having some issues with the tests, if y'all have the time to help I'd greatly appreciate it. Thank y'all for all your help on this! |
Revises #1412
Changes: