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: statement input blocks disappearing #8203

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

BeksOmega
Copy link
Collaborator

The basics

The details

Resolves

Fixes #8189

Proposed Changes

The issue was we weren't properly clearing the state of the current candidate connection, so the preview was incorrect causing blocks to disconnect and disappear, because the place they were trying to be inserted was invalid.

To explain that slightly more linearly:

  1. You start with a nested stack of 3 statement blocks, built from the inside out.
  2. The middle block's current candidate connection is its child, because the state from when it connected to its child was never cleared.
  3. You disconnect the middle block, and it finds the parent as the valid candidate, but the current candidate (its child) is better (I.e. closer) because its state was never cleared. So we use the current candidate instead.
  4. The child blocks gets disconnected and set its parent to null so it can preview, but when that happens it tries to reorder its DOM position, which doesn't work because it's already on the drag layer, where it's not expected to be, causing the error from the original issue.

Test Coverage

Manually tested.

Documentation

N/A

Additional Information

We should put out a patch for this.

@BeksOmega BeksOmega requested a review from a team as a code owner June 10, 2024 22:09
@github-actions github-actions bot added the PR: fix Fixes a bug label Jun 10, 2024
@rachel-fenichel
Copy link
Collaborator

The explanation makes sense, as does the fix. Thanks @FlorianTschimben for the repro steps.

In general, if there's other state the reset between drags, is there an advantage to doing it at the start of the next drag vs at the end of the previous drag? It feels like it's cleanup so I expected it to be at the end of a drag.

@BeksOmega
Copy link
Collaborator Author

The explanation makes sense, as does the fix. Thanks @FlorianTschimben for the repro steps.

In general, if there's other state the reset between drags, is there an advantage to doing it at the start of the next drag vs at the end of the previous drag? It feels like it's cleanup so I expected it to be at the end of a drag.

There isn't any other state we need to set to null between drags. But there is a lot of state we collect at the start of the drag (e.g. the startLoc). So I decided to put it there. If we run into further trouble we can pull it out, but for a patch I'm just going to make the minimal change.

@BeksOmega BeksOmega merged commit dc91c3a into google:develop Jun 11, 2024
10 checks passed
maribethb pushed a commit that referenced this pull request Jun 12, 2024
(cherry picked from commit dc91c3a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disconnecting nested c-shaped blocks causes blocks to disappear
2 participants