-
Notifications
You must be signed in to change notification settings - Fork 13
Refactoring: Function onMouseDown Issues: 29 #45
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: main
Are you sure you want to change the base?
Conversation
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.
It looks good to me, thanks! Just few comments and changes to check and make.
Also, @izhanlaraagarcia use prettier or npm run format to format your code.
| function handleRightClick(event) { | ||
| const hitResult = paper.project.hitTest(event.point, drawingConfig.hitTest); | ||
|
|
||
| if (hitResult?.item.className === 'Path') { |
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.
Please check also if the path is not part of the grid or if we hit a point (those cases would rise an error).
The code below is working fine for me:
if (
hitResult?.type === 'stroke' &&
hitResult?.item.className === 'Path' &&
hitResult.item.parent?.name !== 'grid'
)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.
Hi @pcolt,
This code it's working. You like to change and add your if?
`function handleRightClick(event) {
const hitResult = paper.project.hitTest(event.point, drawingConfig.hitTest);
if (hitResult?.item.className === 'Path') {
selectedLine = hitResult.item;
createContextMenu(event.event);
return true; // The right-click found a target and was handled
}
return false;
}`
`function handleRightClick(event) {
const hitResult = paper.project.hitTest(event.point, drawingConfig.hitTest);
if (
hitResult?.type === 'stroke' &&
hitResult?.item.className === 'Path' &&
hitResult.item.parent?.name !== 'grid'
) {
selectedLine = hitResult.item;
createContextMenu(event.event);
return true;
}
return false;
}`
It's the same result. You say.
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.
Try to right click on a red point or on the empty grid and then click Delete and you should see an error in the console tab
Screencast from 2025-10-08 09-05-36.webm
| paper.view.onMouseDown = function (event) { | ||
| // Handle right-clicks (Guard Clause) | ||
| if (event.event.button === 2) { | ||
| if (handleRightClick(event)) { |
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.
Why are you checking here if the function returns true or false? I would just call the handleRightClick() function here and let the function handle all the cases without any unneeded complexity. If the event is a right click I would always stop here, or I am missing something? (I currently do know better the 3D part of this application)
| createContextMenu(event.event); | ||
| return true; // The right-click found a target and was handled | ||
| } | ||
| return false; // Nothing was done |
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.
Check the comment on line 261 and remove those returns if not needed
|
@pcolt Changes on me local code: Awaing to reply on me comment of: if (
hitResult?.type === 'stroke' &&
hitResult?.item.className === 'Path' &&
hitResult.item.parent?.name !== 'grid'
)After this I push |
|
@izhanlaraagarcia please see my new comment #45 (comment) |
|
@izhanlaraagarcia also remove your changes to file package-lock.json |
|
@pcolt done |
|
Thank you, you fixed the package-lock.json but the other comments are still to be addressed |
Hi everyone,

#29
We have extracted the properties so that they are not repeated since they were repeated several times, in this way we have it centralized and it allows modifications in the future.
We create the right click function:

All code:
