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

fix/reset on zoom and reorder #35

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

Conversation

tschaka1904
Copy link

@tschaka1904 tschaka1904 commented Oct 4, 2022

This fixes #1

It took a little while to figure our what is exactly going on. First thought that the state might be dirty on some point, but this wasn't the issue here.

The solution was behind the opacity slider. When you start moving this one, after reordering, then everything works great.

When we trigger the opacity slider, it is calling: camerasManager.remakeMatrixArgs(store); , which is what reorderMatrixArgs.ts was missing.

Oct-05-2022 01-00-38

@tschaka1904
Copy link
Author

tschaka1904 commented Oct 5, 2022

Whilst this does fix the reordering and the zooming, it does break the custom_label_reorder sorting.

Observation:

  1. Click on a label.
  2. It reorders itself, with the animation.
  3. Click again on a label.
  4. It seems to reorders itself, but without the animation.
  5. Click on any reorder button.
  6. It reorders itself, with the animation.

So something is happening after clicking the reorder button, which fixes it again.

EDIT:
This seems to happen, because the position_arr.ini and position_arr.new are equal, when ordering by row/column. It is working for the general reordering, but not here.

@tschaka1904 tschaka1904 marked this pull request as draft October 6, 2022 15:47
@tschaka1904 tschaka1904 marked this pull request as ready for review October 6, 2022 15:48
@moromis moromis self-requested a review October 20, 2022 21:26
@moromis
Copy link

moromis commented Oct 20, 2022

Whilst this does fix the reordering and the zooming, it does break the custom_label_reorder sorting.

Observation:

  1. Click on a label.
  2. It reorders itself, with the animation.
  3. Click again on a label.
  4. It seems to reorders itself, but without the animation.
  5. Click on any reorder button.
  6. It reorders itself, with the animation.

So something is happening after clicking the reorder button, which fixes it again.

EDIT: This seems to happen, because the position_arr.ini and position_arr.new are equal, when ordering by row/column. It is working for the general reordering, but not here.

So what do we want to do about this? Would it be best to leave this for another PR as it is a separate issue and make an issue for it?

@moromis
Copy link

moromis commented Oct 20, 2022

Fantastic find by the way, great job scoping this PR to the minimal change needed to fix this issue. If this is all that should be in this PR then I'll approve it, but if adding it broke the custom label ordering then we should address that. Let me know.

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.

Matrix cells reset on zoom/reorder
2 participants