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

Accudraw update fixes #1157

Merged
merged 17 commits into from
Jan 7, 2025
Merged

Accudraw update fixes #1157

merged 17 commits into from
Jan 7, 2025

Conversation

mathieu-fournier
Copy link
Contributor

@mathieu-fournier mathieu-fournier commented Dec 13, 2024

Changes

This PR goes hand in hand with some fixes that have been made in Core-frontend : itwinjs-core/pull/7475.
This stabilizes AccuDraw behaviours in general.

Fixes :

1- When moving the mouse, the focus only change between X or Y in rectangular mode only.

2- Revereted that fix, it will need to be done in another PR. See the comments. Only updating input fields value when their state is dynamic. Reason : onMotion is supposed to be called way more often, like on every key typed. This prevents the input field from updating at the wrong moment. (Works with the core-frontend fix)

3- Removed the delay after typing in the input field, the visual update should be immediate. (Works with the core-frontend fix)

4- Input fields are automatically focused when the AccuDrawFieldContainer appears and the the compass mode changes.
image

Improvements :

5- Rectangular inputs no longer have colors.

6- We can enter letters in the Bearing angle input field. N, S, E, W for North, South, East and West.

7- Bearing angle input field automatically adds special characters ° ' " to facilitate entering bearing angles.

8- The Focus is now trapped in Accudraw input fields. To focus out of these fields, the user must press Escape.

Thanks (:
Mathf

@mathieu-fournier mathieu-fournier requested a review from a team as a code owner December 13, 2024 21:27
@GerardasB GerardasB added the minor Changes in this PR requires creating a minor release label Dec 16, 2024
Copy link
Collaborator

@GerardasB GerardasB left a comment

Choose a reason for hiding this comment

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

Additionally, I'd appreciate if we could update (in separate PR) AccuDraw.md to actually describe what accu draw is and how it is expected to work.

@mathieu-fournier
Copy link
Contributor Author

Additionally, I'd appreciate if we could update (in separate PR) AccuDraw.md to actually describe what accu draw is and how it is expected to work.

I'll do it in another PR, AccuDraw doc is probably already in Core. I'll link this doc and make a fresh doc for FrameworkAccuDraw

@bbastings
Copy link
Contributor

AccuDraw doc is probably already in Core.

@mathieu-fournier There's an AccuDraw section in the learning docs (PrimitiveTool.md) but since there's no UI in core it's pretty bare bones.

@mathieu-fournier mathieu-fournier merged commit fee652f into master Jan 7, 2025
18 of 20 checks passed
@mathieu-fournier mathieu-fournier deleted the accudraw_update_fixes branch January 7, 2025 13:26
@mathieu-fournier
Copy link
Contributor Author

AccuDraw doc is probably already in Core.

@mathieu-fournier There's an AccuDraw section in the learning docs (PrimitiveTool.md) but since there's no UI in core it's pretty bare bones.

Should we put the doc in core or appui ?

@bbastings
Copy link
Contributor

@mathieu-fournier Document in appui so you can reference/link ui classes, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Changes in this PR requires creating a minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants