Add block tap listeners

[tmickel]

This adds listeners to the workspace for detecting when a block is tapped. We can register callbacks with the workspace, and they are called with the block ID and the root block ID (representing the stack). We can then pass this along to our VM/runtime for starting the stack that starts with the root block, or showing data associated with the block in the case of a reporter.

Taps are triggered by the block's mouseup_ (or touchend) events, when the drag hasn't been moved outside the DRAG_RADIUS (i.e., the block hasn't moved).

@rachel-fenichel

[rachel-fenichel]

Does this also work in the flyout? I think it should, since that's just another workspace, but want to confirm.

This will conflict slightly with my refactor but should be easy to reconcile.

[tmickel]

@rachel-fenichel great question. Right now it only works with the main workspace (Blockly.mainWorkspace.flyout_.workspace_.addTapListener does not appear to work either). I'm not sure why it doesn't work with flyout.workspace - I'm guessing that onMouseUp events aren't fired there.

I'm thinking I should extend Blockly.Flyout.prototype.onMouseMoveBlock_ for that purpose, and bubble up the event to targetWorkspace_. In that case I'd like to not pass the block ID but instead pass the block's name. Does this approach make sense to you or would you suggest something different?

[rachel-fenichel]

Whether to pass the block name or ID probably depends on how we're doing events in the flyout--that is, how ephemeral those blocks are. If those blocks will be added once and nothing happens to them when their particular tab of the flyout is opened or closed, block ID should work. What's that advantage of using block name?

In the flyout targetWorkspace_ is the workspace that blocks will eventually end up in. Unconditionally bubbling that event up is almost certainly going to have weird side effects.

For working in the flyout, the block listeners are set here and behaviour depends in part on whether the flyout is closeable.

[tmickel]

@rachel-fenichel Regarding names/IDs: my concern is that a Scratch VM will have no idea what a block ID from the flyout represents. Since Blockly's "create" events only seem to happen for blocks that are not in the flyout, we'll need to do some kind of lookup to tell what the desired operation for the block ID is. However I guess we should keep in mind that eventually we'd like to be editing field values in the flyout, etc., so maybe a lookup is actually most appropriate.

Thanks for addBlockListeners_ - to me that verifies that blockMouseDown_ should be where we put a tap event. If we ever have a closeable flyout, I don't think a tappable block in the flyout makes that much sense (as it turns out, that already works because a new block is created and "tapped").

Where do you suggest we send an event instead of bubbling to targetWorkspace_? I'm not immediately thinking of the weird side effects.

[rachel-fenichel]

I think you would want to add a mouseUp (like you have in the main
workspace) instead of doing it on mouseDown_.

The block you would be tapping isn't actually in the target workspace, so
it's a bit concerning to me to send an event through that workspace. I
would almost rather have the tap listener be added on the workspace in the
flyout (even if it's the same tap listener) just to keep them properly
separated.

That said, I also can't list of specific side effects so maybe it's a
nonissue.

Separately, addBlockListeners_ has duplicated listeners, here and in
mainline blockly, which looks like an old mistake.

On Wed, Feb 24, 2016 at 12:16 PM Tim Mickel notifications@github.com
wrote:

@rachel-fenichel https://github.com/rachel-fenichel Regarding
names/IDs: my concern is that a Scratch VM will have no idea what a block
ID from the flyout represents. Since Blockly's "create" events only seem to
happen for blocks that are not in the flyout, we'll need to do some kind of
lookup to tell what the desired operation for the block ID is. However I
guess we should keep in mind that eventually we'd like to be editing field
values in the flyout, etc., so maybe a lookup is actually most appropriate.

Thanks for addBlockListeners_ - to me that verifies that blockMouseDown_
should be where we put a tap event. If we ever have a closeable flyout, I
don't think a tappable block in the flyout makes that much sense (as it
turns out, that already works because a new block is created and "tapped").

Where do you suggest we send an event instead of bubbling to
targetWorkspace_? I'm not immediately thinking of the weird side effects.

—
Reply to this email directly or view it on GitHub
#84 (comment).

[tmickel]

Ah yes mouseUp definitely!

Ok - I'll keep them separated. I'll expose a method on the main workspace that is "addFlyoutTapHandler" or similar. And for now I'll send both the block ID and the block name until we figure out how we want to do editing in the flyout/querying those changes.

[rschamp]

This LGTM but I think @rachel-fenichel may be the more appropriate reviewer.

[rachel-fenichel]

I merged my stuff so you now have a small conflict, but lgtm after that.

[tmickel]

Merging with the note that we should move it to Neil's UI event system once he does that.