-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Update docs for custom shapes and related features #7734
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
base: dev-2.0
Are you sure you want to change the base?
Conversation
…v-2.0 Update cloned repo
…of vertex.js with custom_shapes.js
…with custom_shapes.js
Hi @GregStanton ! Quick update here: I've added minimal documentation and fixes to example code here: #7749 Please feel free to revise the text in any way that makes sense - though I believe everything is in there, some points could be much more clear. Thank so much for your work on this! |
Sounds good @ksen0! Thank you so much for putting those docs together. Since all tests are passing in this code reorg, would it make sense for me to copy over your docs, and then make any revisions to them here? I'll hold off on that for now, in case it's easier for you or @davepagurek to review the reorg by itself, without extra commits. Future beginner-friendly issue to improve test coverage and structureMaybe after the docs are updated, we could make an issue with the "Good First Issue" and "Help Wanted" labels, so that we can give beginners an opportunity to improve the coverage and structure of tests for custom shapes? (Maybe it could work as a good first issue if we provide some support in case reorganization breaks things.) I'll record a few notes for that potential issue below, before I forget what the problems are. I wouldn't mind if you skip them for now 🙂 CoverageOne thing I noticed when updating imports and tests is that we seem to be lacking full test coverage in The 1.x version only has tests for StructureThere's also some unnecessary friction for contributors, since the file structure of the unit-test directory doesn't reflect the new file structure for 2.0. For example, under |
@GregStanton copying the updated docs to your branch sounds like the right next step! if you have the time, feel free to start that process, and I'm also happy to help after the Easter weekend too! I think I should be OK reviewing regardless of the extra commits. |
Hi both! @GregStanton yes that seems like a great way forward. Please don't feel like you need to keep any of the text I added, you're welcome to organize it as best makes sense. Merging the dev-2.0 branch into this might be a challenge, I see there's some merge conflicts, so please feel free ping if you need help with that, either on Discord or via public office hours! Test coverageYes that would be great! Your notes on coverage make total sense. I support reorganization to improve how navigable the codebase it, but for bulk changes (including on test suite), it would be better to keep reorg separate from substantive changes. Additionally, I think it might be good to include recent bugs that were patched that touch the Shape API (e.g., #7750) as potential inspiration of what a test might cover. Finally, a rule of thumb is that if a function was moved from one file or another, just make sure there's 1 more meaningful test added. Really important these aren't AI-generated, because a good test suite covered both very typical and sort of odd cases, and AI might generate OK typical cases, but is unlikely to generate good strange cases. If I understand all this right, the test coverage issue can already be open and available for work - it's totally independent of this documentation work? |
Issues
Addresses #6560
Addresses #6766
Pull requests
Changes
This PR updates docs for user-facing changes to custom shapes and related features.
Custom shapes:
vertex()
docstexture()
, but not yet documented on ref page forvertex()
bezierOrder()
quadraticVertex()
and move them herebezierVertex()
docsx, y, u, v
andx, y, z, u, v
signaturesbezierOrder()
?quadraticVertex()
docs- relevant PRs: #7373, 7600
- relevant Issue: #6766
splineVertex()
curveVertex()
docssplineProperty()
and link to it?)curveVertex()
(it's already supported by the 1.x compatibility add-on for shapes)vertexProperty()
docssplineProperty()
vertexProperty()
,curveTightness()
splineProperties()
beginShape()
docsPATH
is the new name for the default shape kindbeginContour()
docsendShape/Contour(CLOSE)
behavior for splinessplineVertex()
?)endContour(CLOSE)
docsOPEN
)Curves:
curve()
docs to reflect name change tospline()
curveTightness()
docscurvePoint()
docs to reflect name change tosplinePoint()
curveTangent()
docs to reflect name change tosplineTangent()
curveDetail()
docs, possibly incorporating adapted examples frombezierDetail()
docsbezierDetail()
docs3D:
beginGeometry()
/endGeometry()
docsbuildGeometry()
.beginGeometry()
/endGeometry()
eliminates an inconsistency withbeginShape()
/endShape()
, reduces the size of the user-facing API, and prevents confusion (endGeometry()
is effectively used as a constructor, in contrast with how objects are usually created in p5)."Notes
endShape(CLOSE)
for Bézier curves was discussed; decided we can implement this for 2.0Screenshots
[Reminder] Might be helpful to provide screenshots of top-level listings from main reference page.
PR Checklist
npm run lint
passesFootnotes
Here, reference dependency refers to a reference page that depends on one of the new or changed features mentioned under the Changes section, either because it mentions it or because it uses it in a code example. ↩