Skip to content
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

Angle snapping tolerance functionality #5945

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mohsenD98
Copy link
Collaborator

@mohsenD98 mohsenD98 commented Jan 15, 2025

PR Description
The snapping to angles functionality was working, but the snapping tolerance (the distance within which the cursor would snap to an angle) was too low. This low tolerance made it difficult to snap accurately to angles, especially when digitizing without a mouse. The precision required to hit the right angles was cumbersome, causing a less intuitive experience for users.

Solution:
To address this, we added a new option that allows users to configure the snapping tolerance for angles. This change improves usability by giving users more flexibility in setting the snapping distance to their preference, making it easier to snap to angles accurately without requiring precise cursor placement.

Changes Made:

  • Introduced a configurable snapping tolerance setting for angle snapping.
    • Narrow
    • Normal
    • Large
  • Users can now adjust the snapping distance to better suit their needs.

This enhancement improves the overall user experience, especially when precision is important.

Image:

image

@mohsenD98 mohsenD98 requested a review from nirvn January 15, 2025 16:16
@mohsenD98 mohsenD98 self-assigned this Jan 15, 2025
@mohsenD98 mohsenD98 changed the title QF-5055 **NEW** snapping to every angle and tolerance user interface. QF-5055 Snapping to every angle and tolerance config. Jan 15, 2025
@nirvn nirvn closed this Jan 15, 2025
@mohsenD98 mohsenD98 reopened this Jan 31, 2025
@mohsenD98 mohsenD98 force-pushed the snapping-angle-configure branch from df6f764 to aa21c33 Compare January 31, 2025 09:55
@qfield-fairy
Copy link
Collaborator

qfield-fairy commented Jan 31, 2025

@mohsenD98 mohsenD98 force-pushed the snapping-angle-configure branch from 6571111 to c3bcd17 Compare January 31, 2025 12:18
@mohsenD98 mohsenD98 marked this pull request as ready for review January 31, 2025 14:10
@mohsenD98 mohsenD98 force-pushed the snapping-angle-configure branch from c3bcd17 to f434250 Compare January 31, 2025 14:11
@mohsenD98 mohsenD98 force-pushed the snapping-angle-configure branch from f434250 to b8d7c3d Compare January 31, 2025 14:13
@nirvn nirvn changed the title QF-5055 Snapping to every angle and tolerance config. Angle snapping tolerance functionality Feb 2, 2025
@nirvn
Copy link
Member

nirvn commented Feb 2, 2025

Alright, first, UI-wise, I think we need to make sure we have our text labels aligned vertically, which means you'll need to push add padding to pretty much everything there, i.e.:

image

Also, I think we've discussed this briefly but just as a recall, I think we should not close the popup when someone clicks on elements of the popup as I think it'd be quite likely a user would want to change more than one setting at a time.

Onto the actual functionality, I see there's a problem that needs resolving: when tolerance is set to largest, snapping doesn't occur when your coordinate cursor gets close to the last entered point. It's actually something that's happening even at normal snapping tolerance, but it's most visible when set at large.

Looking at the code now.

Comment on lines 404 to 415
case 0 // Narrow
:
return 0.5;
case 1 // Normal
:
return 1;
case 2 // Large
:
return 4;
default:
return 1;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouf, what's happening with the formatting here? Is it caused by the // comment next to the case forcing the column (:) to go onto next line? Let's fix that.

also, you should have case 1 and default both be together. The logic here is that default == normal. If we keep them separated, it's quite likely we'll end up changing one value and forget the other.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes formatting did this, Agreed 👍

Add padding, fix typo, fix formatting, try to fix logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants