3-point circle interactive component #4982
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
lf94
commented
Jan 9, 2025
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4982 +/- ##
==========================================
- Coverage 85.99% 85.96% -0.03%
==========================================
Files 88 88
Lines 31470 31480 +10
==========================================
Hits 27062 27062
- Misses 4408 4418 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
jtran
reviewed
Jan 10, 2025
jtran
reviewed
Jan 10, 2025
jtran
reviewed
Jan 10, 2025
jtran
requested changes
Jan 10, 2025
franknoirot
reviewed
Jan 10, 2025
There was a problem hiding this comment.
Looking good! Just a few notes:
- The click target for dragging seems very small, is there some way to make it more forgiving?
- The "snap to axis" draft points appear if the user mouses over the axes even after the circle has been added.
- I think we talked about adding the constraint overlays in a follow-up PR, just want to note it
- And I just want to note that it's a known issue that the circle button doesn't activate while this tool is active, same issue faced by center rectangle and will be addressed elsewhere
lf94
force-pushed
the
lf94/circle-3-point-drag
branch
from
009f654 to
3a71837
Compare
Uses our talked about technique of calling Rust functions to calculate new geometry coordinates and parameters. It works very well! Need to have the modeling app show "edit sketch" still.
lf94
force-pushed
the
lf94/circle-3-point-drag
branch
from
3a71837 to
aab4d91
Compare
|
@franknoirot I fixed points 1 and 4 - I think 2 is related to a class of bugs around Circle in general which we can fix later (keeps axis indicator up). |
pierremtb
approved these changes
Jan 16, 2025
jtran
approved these changes
Jan 16, 2025
TomPridham
pushed a commit
to TomPridham/modeling-app
that referenced
this pull request
Feb 13, 2025
* Add dragging behavior to 3 point circle Uses our talked about technique of calling Rust functions to calculate new geometry coordinates and parameters. It works very well! Need to have the modeling app show "edit sketch" still. * Cargo fmt * cargo fmt * Address Jon's comments * Fix clippy * Dont use isNaN * Make points easier to select (enlarge) * Fix circle button not being activated * Ensure efficiency of updating editor vs execution * Make cargo clippy happy
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
2025-01-08_20-54-28.mp4