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

Fixing block deletions problems with some sticky comments #2568

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jguille2
Copy link
Contributor

@jguille2 jguille2 commented Apr 2, 2020

Reported at forum bug reports.

Fixing this issue I saw that there were three related problems:

  • Deleting the first (only the first) block of a C input when there are other blocks in that C-input with comments.
  • Deleting the whole script (C-block) with comments inside
  • Similar actions done by keyboard edition.

At code:

  • There were three prepareToBeGrabbed calls without params, and this (the missing hand param) causes the problem (it is needed when threre are comments, to trigger starFollowing function).
  • Fixing this, appears another problem, because this triggering action starts comments startFollowing. But this action (shadows and more) are stopped by the drop event... and here we have no drop event. So, I've changed these functions adding an allAtOnce param to control this behavior.

That's all!
Joan

@jmoenig
Copy link
Owner

jmoenig commented Apr 23, 2020

Thanks, Joan! This is a somewhat hairy issue, let me think about this some more. I'll most likely pull this along with the others for the next release.

jmoenig added a commit that referenced this pull request Apr 29, 2020
@jmoenig
Copy link
Owner

jmoenig commented Apr 29, 2020

Hi @jguille2, I just came across this issue (or is it just a similar / related one?) as I was going through the blocks.js code to refactor it. So I thought about the quickest way to fix it and came up with this: e74c95a
This seems to fix it for me. What do you think?
Thanks!

@jguille2
Copy link
Contributor Author

Hi @jmoenig ,

The problem of this solution is the second point I detected:

  • There were three prepareToBeGrabbed calls without params, and this (the missing hand param) causes the problem (it is needed when threre are comments, to trigger starFollowing function).
  • Fixing this, appears another problem, because this triggering action starts comments startFollowing. But this action (shadows and more) are stopped by the drop event... and here we have no drop event. So, I've changed these functions adding an allAtOnce param to control this behavior.

You can see the the "attaching line" of the comment turns to grey (behavior of a drag and drop action) and that state (that "startFollowing" state without stopping) causes other IDE crashes.

This is the reason I had to add this "allAtOnce" param, because I see the missing "hand", but the real problem is that we don't want to "startFollowing" because we can't stop it (he have no "drop" event).

Joan

@jmoenig
Copy link
Owner

jmoenig commented Apr 29, 2020

Ah, okay, so there's more to the issue than I suspected, thanks for the explanation, @jguille2 !
I'll keep this PR open and integrate it along with the others then...

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