Skip to content

Refactor/IHprLayout #10796

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

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

JassonCordones
Copy link
Contributor

@JassonCordones JassonCordones commented Jun 20, 2025

Describe your PR, what does it fix/add?

Split the performSnap and onMouseMove function to make it easier to test and work with.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

no

Is it ready for merging, or does it need work?

yes, is ready I don't want to make the PR too big

@JassonCordones JassonCordones marked this pull request as ready for review June 21, 2025 14:35
@JassonCordones JassonCordones changed the title Refactor/ihprlayout perform snap Refactor/IHprLayout Jun 22, 2025
…llbacks to avoid flooding"

This reverts commit 318688e.
@JassonCordones
Copy link
Contributor Author

@vaxerski This refactor can be merged.

@vaxerski
Copy link
Member

No, I don't think so. What does this refactor achieve? There are no benefits functionally, only potential (and wouldn't be the first time) regressions. You're rewriting a large part of code that has had historically dozens of small edge cases and issues, in a huge unreviewable dollop.

In general, I am not in favor of huge refactors for no gain, only because "uhh code looks complicated".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants