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

Adds minimum block height #2522

Merged
merged 2 commits into from
May 30, 2019
Merged

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

Problem with some rows not having the correct padding on the bottom row.

Proposed Changes

Add height to the bottom row if the current height does not meet the minimum heigh requirement.

Reason for Changes

Test Coverage

Tested on:

Additional Information

@@ -639,6 +639,12 @@ Blockly.BlockRendering.RenderInfo.prototype.finalize_ = function() {
this.blockBottom = yCursor;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Check whether we're still using blockBottom. If we are, your change to the yCursor needs to be above this line. If not, add an item to the gnosis to delete it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't look like we are using it. Will add a task for cleanup in gnosis.

@rachel-fenichel
Copy link
Collaborator

Looks good, please check why the tests are failing.

@alschmiedt
Copy link
Contributor Author

Ran tests locally and both passed. Looks like it is not related to this change so will merge and continue investigating.

@alschmiedt alschmiedt merged commit bd33b0a into google:render/collab May 30, 2019
@alschmiedt alschmiedt deleted the bottom_padding branch May 29, 2020 15:34
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