-
-
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
Patch-CORNER-descr-cx #7062
Patch-CORNER-descr-cx #7062
Conversation
🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page! |
src/core/shape/attributes.js
Outdated
@@ -15,7 +15,7 @@ import * as constants from '../constants'; | |||
* By default, the first two parameters of | |||
* <a href="#/p5/ellipse">ellipse()</a>, <a href="#/p5/circle">circle()</a>, | |||
* and <a href="#/p5/arc">arc()</a> | |||
* are the x- and y-coordinates of the shape's center. The next parameters set | |||
* are the x- and y-coordinates of the shape's upper left corner. The next parameters set |
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 do you know if this one needed correcting? Looks like it's the center in the current docs https://p5js.org/reference/#/p5/ellipse so ellipse
might be good as is
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 per my tests, the x and y coordinates of ellipse() determine the position of the center of the figure by default, so its description is accurate.
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.
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.
No problem! If you run git revert 4892c6f
and push the results, then the Files Changed tab on the PR will only show the changes from cda061b.
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 I run that command somewhere from GitHub? I used Codespaces to make those commits. Can you please send me some quick steps on how to do it? Thanks for the help
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.
Ah looks like you got 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.
Yeah I think so lol. Is there something else I should do?
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 think it should be good, I'm just waiting for tests to pass and then I'll merge if they look good! (and they should since its just a change to the reference text.)
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! It is a very simple contribution, but because it is my first in some way it is special for me haha. Thanks!
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.
Thanks, we appreciate it!
This reverts commit 4892c6f.
@all-contributors please add @JulioGitLab for docs |
I've put up a pull request to add @JulioGitLab! 🎉 |
Resolves #[Add issue number here]
Changes: Corrects the description of the rectMode() CORNER attribute
Screenshots of the change:
PR Checklist
npm run lint
passes