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

Refactors bottom row to be a row instead of measurable #2478

Merged
merged 4 commits into from
May 23, 2019

Conversation

alschmiedt
Copy link
Contributor

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

Proposed Changes

Turns the bottom row into a row so that it can hold elements such as corner and next connection.

Reason for Changes

Test Coverage

Tested on:

Additional Information

Copy link
Collaborator

@rachel-fenichel rachel-fenichel left a comment

Choose a reason for hiding this comment

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

Few nits but lgtm

@@ -188,6 +190,25 @@ Blockly.BlockRendering.RenderInfo.prototype.createTopRow_ = function() {
}
};

/**
* Create the top row and fill the elements list with all non-spacer elements
Copy link
Collaborator

Choose a reason for hiding this comment

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

top -> bottom

@@ -237,7 +258,7 @@ Blockly.BlockRendering.RenderInfo.prototype.addElemSpacing_ = function() {
var oldElems = row.elements;
row.elements = [];
// No spacing needed before the corner on the top row.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Update comment.

@@ -341,13 +366,17 @@ Blockly.BlockRendering.RenderInfo.prototype.getInRowSpacing_ = function(prev, ne
}

// Spacing between a rounded corner and a previous connection
Copy link
Collaborator

Choose a reason for hiding this comment

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

Update comment, here and below.

// If we have an empty block add a spacer to increase the height
if (prev.type === 'top row' && next.type === 'bottom row') {
return BRC.EMPTY_BLOCK_SPACER_HEIGHT;
}
// Top row acts a spacer so we don't need any extra padding
Copy link
Collaborator

Choose a reason for hiding this comment

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

Update comment to describe bottom row as well.


this.hasPreviousConnection = !!block.previousConnection;
this.connection = block.previousConnection;

var precedesStatement = block.inputList.length &&
block.inputList[0].type == Blockly.NEXT_STATEMENT;

//
// This is the minimum height for the row. If one of it's elements has a greater
Copy link
Collaborator

Choose a reason for hiding this comment

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

it's -> its

@alschmiedt alschmiedt merged commit cf793ca into google:render/collab May 23, 2019
@alschmiedt alschmiedt deleted the refactor_bottomRow branch May 29, 2020 15:36
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.

None yet

2 participants