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

update(AligningGuidelines): Fix some bugs and add custom features. #10120

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

Conversation

zhe-he
Copy link
Contributor

@zhe-he zhe-he commented Sep 2, 2024

Resolve the issue where when the user scales using scaleX, scaleY is not updated simultaneously. The same applies to scaleY.

Copy link

codesandbox bot commented Sep 2, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@zhe-he
Copy link
Contributor Author

zhe-he commented Sep 2, 2024

2024-09-02.14.23.03.mov

test

@zhe-he zhe-he force-pushed the fix-aligning-guidelines-uniformScaling branch from 5e1acc1 to 326e756 Compare September 10, 2024 09:09
@zhe-he zhe-he force-pushed the fix-aligning-guidelines-uniformScaling branch from 326e756 to 9523d69 Compare September 10, 2024 09:19
@zhe-he
Copy link
Contributor Author

zhe-he commented Sep 10, 2024

When the angle of the operated graphic is not 0, there is a bug with the 4-corner scaling alignment.

I will solve it at noon tomorrow.

@zhe-he
Copy link
Contributor Author

zhe-he commented Sep 11, 2024

fixed

2024-09-11.10.19.10.mov

demo

When the angle of the operated graphic is not 0, there is a bug with the 4-corner scaling alignment.
I will solve it at noon tomorrow.

@zhe-he
Copy link
Contributor Author

zhe-he commented Sep 23, 2024

In practical use, a common issue has been identified: when there are too many shapes, the problem of excessive reference line hits occurs because each shape is compared with the current shape. It is suggested to change the approach to count all points and use those points for judgment. (The same issue occurs in V5; if there are related requirements for V5, please refer to the modifications.)

The following is a video comparison before and after the modifications.

a.mov
b.mov

@asturur
Copy link
Member

asturur commented Sep 23, 2024

I m sorry i didn't have time for this and i won't for a bit.
All the implementation needs to be checked and verified a bit imho.
When there are too many lines there must be a way for the developer to choose the one they care about or expose an interaction key to manually disable them. Is a common problem

@zhe-he
Copy link
Contributor Author

zhe-he commented Sep 24, 2024

Okay, I will take some time to write the test cases, while also adding the corresponding listener functions and disabling the relevant feature switches.

@zhe-he
Copy link
Contributor Author

zhe-he commented Sep 24, 2024

@asturur
I made a lot of updates, and the final result is significantly different from the initial version. You can ignore the logic of the initial version and directly look at all the final code. Compared to the initial version, I added comments to each line, making it easier to understand.
Additionally, I added user customization options, which are documented in the README. Users can manually disable horizontal lines and customize the alignment of graphics. It also supports user-defined names for controllers.
If you have any questions about any line of code, feel free to tag me, and I will explain once I see it.
This is the final version. If you have time, please download the code locally to review all files; it will be faster than reviewing changes on GitHub. Thank you.
Regarding the test cases for simulating user manual dragging, I haven't written any before. If needed, I will try to add them.
Here is a test demo where you can test by modifying parameters or customizing them.
demo

@zhe-he zhe-he changed the title fix(AligningGuidelines): Align guidless changes aspect ratio on snapping when scaling update(AligningGuidelines): Fix some bugs and add custom features. Sep 24, 2024
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.

2 participants