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: parent blocks not bumping neighbours #6538

Merged
merged 2 commits into from
Oct 13, 2022

Conversation

BeksOmega
Copy link
Collaborator

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

#6017

Proposed Changes

makes it so that whenever a block in a block stack has bumpNeighbours called on it, all of the connections in the block stack bump.

We have to bump every single connection, because with custom renderers, we can't predict how blocks will be shaped based on other blocks. E.g. in geras higher sibling connections don't move, but in other renderers they might. So we have to trigger bumping on everything.

Behavior Before Change

Parent blocks would not bump neighbours when a child block was rerendered.

Behavior After Change

Parent blocks do bump neighbours when a child block is rerendered.

Reason for Changes

Having blocks appear tto be connected (visually) without actually being connected (in terms of code generation) can cause confusing results and frustration for end-users. This helps eliminate that.

Test Coverage

N/A

Documentation

N/A

Additional Information

Also did some code cleanup of bumpNeighbours while I was in the area.

@BeksOmega BeksOmega requested a review from a team as a code owner October 12, 2022 21:58
@BeksOmega BeksOmega requested a review from maribethb October 12, 2022 21:58
@github-actions github-actions bot added the PR: fix Fixes a bug label Oct 12, 2022
const neighbours = connection.neighbours(config.snapRadius);
for (let j = 0, otherConnection; otherConnection = neighbours[j]; j++) {
const renderedOther = otherConnection as RenderedConnection;
// If both connections are connected, that's probably fine. But if
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment in particular and probably the other ones are helpful context to leave in.

@BeksOmega BeksOmega merged commit 7147813 into google:develop Oct 13, 2022
@BeksOmega BeksOmega deleted the fix/bump-neighbours branch October 13, 2022 17:59
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.

2 participants