Flyout doesn't match intended design

[drigz]

If you load the default toolbox, eg with tests/horizontal_playground.html, you see a flyout as below:

This has the following issues:

• The top margin is too small.
• The blocks are not aligned as they would be if they were stacked together.

The intended design is shown in this screenshot from the blog post:

[Tymewalk]

Look at line 1145 of core/flyout.js (I don't know how to link lines in GitHub 😦).

Through testing, this.margin is the margin on the bottom - when it changes, the blocks stay the same.
This leads me to believe that the blocks aren't being centered inside the flyout.

[Tymewalk]

Still new to this so correct me if I'm wrong.

Lines 1156 - 1174 in core/flyout.js control where the block gets placed within the flyout:

if (this.height_ != flyoutHeight) {
    for (var x = 0, block; block = blocks[x]; x++) {
      var blockHW = block.getHeightWidth();
      if (block.flyoutRect_) {
        block.flyoutRect_.setAttribute('width', blockHW.width);
        block.flyoutRect_.setAttribute('height', blockHW.height);
        // Blocks with output tabs are shifted a bit.
        var tab = block.outputConnection ? Blockly.BlockSvg.TAB_WIDTH : 0;
        var blockXY = block.getRelativeToSurfaceXY();
        block.flyoutRect_.setAttribute('y', blockXY.y);
        block.flyoutRect_.setAttribute('x',
            this.RTL ? blockXY.x - blockHW.width + tab : blockXY.x - tab);
      }
    }
    // Record the width for .getMetrics_ and .position.
    this.height_ = flyoutHeight;
    // Top-most block.  Fire an event to allow scrollbars to resize.
    Blockly.asyncSvgResize(this.workspace);
  }

Specifically these lines:

block.flyoutRect_.setAttribute('height', blockHW.height);
...
block.flyoutRect_.setAttribute('y', blockXY.y);

These lines look like they control how the block is placed in the flyout.

If we used this:

block.flyoutRect_.setAttribute('height', flyoutHeight);

It should make the blocks center properly (I think).

Testing now.

EDIT: I was wrong. That controls the size of the rect that the block is in - not where it's placed in the flyout.

[rachel-fenichel]

Hey there! There's a big merge coming in soon that may fix some problems: google/blockly#291 makes the horizontal toolbox much more robust and deals with RTL properly, among other things.

I'll be merging google/develop in again soon (hopefully today or tomorrow), so line numbers will change significantly soon.

I think the problem with aligning by baseline will still be present post-merge. The top margin may be correct.

[drigz]

Thanks for looking at this, @Tymewalk! Here are the steps I follow to link to a line:

1. Go to file on GitHub webview, eg https://github.com/LLK/scratch-blocks/blob/develop/core/flyout.js
2. Press 'y' to make the URL a permalink: should look like https://github.com/LLK/scratch-blocks/blob/af963c9a0ed63a792fe189762a419c66fc01c485/core/flyout.js (note the hash)
3. Either click the line number, or add #L.... to the URL, eg https://github.com/LLK/scratch-blocks/blob/af963c9a0ed63a792fe189762a419c66fc01c485/core/flyout.js#L1156

It would be nice if GitHub did this automatically when you start typing something like core/flyout.js:1156, just like it helps you with PRs and issues when you type # or names when you type @...

[rachel-fenichel]

Merge PR is #350

The baseline problem is definitely still present.

[thisandagain]

Tracking in #21