Add context menu items to facilitate editing

[cpcallen]

Add the following items to the context menus for blocks, with the the given keyboard shortcuts:

• Edit contents (→): Moves the cursor (focus) to the first focusable field or input.
  • This option exists mainly to facilitate discoverability of the right arrow as a means to navigate to the contents of a block. It should probably be the first item in the context menu.
• Move block (m): "Picks up" the block to move it using the dragging workflow (see Make it possible to move blocks on the workspace #106).
• Replace block (TBD, perhaps replace or place?): use the marker workflow to place a new block at this location. #173
  • See Make placing blocks using the marker more intuitive #102 for details of how this should work.
• Delete block (del+backspace): delete the block, as current delete shortcut does.
• Cut (⌘X etc)
• Copy (⌘X etc)
• Paste (⌘X etc): Paste (see Make paste behaviour more intuitive #128).

Add the following item to the context menu for unconnected next and input connections, as well as when the cursor is visiting the workspace:

• Place block (same as replace block, above): place a new block at this location using the marker workflow.

The names given above are not necessarily intended to be the final English names for the given items, merely descriptive.

[cpcallen]

@rachel-fenichel: as I noted in the discussion with Kirsty, I think "Insert" should be the primary shortcut for adding blocks, and I'm not sure that if we have insert we need replace.

[kmcnaught]

Is this a question about naming, shortcut key choice, or functionality? Or all three?

If you are at a prev/next connection then "insert" would add a new block in the current location.

If you were at an in/out connection (not a block) then an action called "insert" might replace the currently-connected block(s) with the newly selected one. Are there any geras-style platforms where it might be appropriate/meaningful to insert a block between in/out as an extra block, without replacing anything? I don't think this would ever be the case in MakeCode.

If you were at a [single cursor location that collapses a block and its out connection] then I was imagining an action called "replace" would replace the currently-connected block(s) with the newly selected one.

What we can't rely on is using "delete" followed by "insert" to work for reporter blocks as an alternative for "replace", since deleting might (nearly always in MakeCode) leave you with a shadow block. So for blocks connected at an in/out connection, there needs to be an action that is semantically equivalent to "replace" even if you decide it's unambiguous to call it "insert" (I think it's much easier to argue this at an in/out connection than at a "collapsed single location".)

For instance, if we collapse to a single cursor location (as per Matt's demo) then in the below situation, I would expect a context menu to give me a single action for "insert/replace" that replaces the block.

I can see an argument for keeping the naming/shortcut keys equivalent for these actions, and I can't see a situation where "Insert" and "Replace" would be 2 different valid actions, unless there's some geras-style applications where inserting in the middle of a horizontal stack of blocks would be desirable

[rachel-fenichel]

We discussed this live today with @cpcallen, @microbit-matt-hillsdon, Lucy, Emma, and Robert. I think our conclusions were:

• All of the insertion/replacement flows you described need to be possible (so we seem to agree about required functionality)
• The name of the action, as shown to the user, may be different in different places (e.g. insert vs replace)
• Replacement is more likely in horizontal cases, while insertion is more likely in vertical cases
• Christopher objects to having both "Insert (above)" and "Replace" on a stack block, finding them redundant
• Many of these cases are handled in core Blockly in the dragging logic, and Christopher and Rachel think we should match that logic where possible
  • Rachel also wants this because it means free functionality by just calling connect

In #196 I implemented the necessary logic for replacing a reporter block (block with an output) with an appropriate block from the flyout/toolbox.

I agree that in your case above, the context menu has an insert/replace action.

[rachel-fenichel]

Are there any geras-style platforms where it might be appropriate/meaningful to insert a block between in/out as an extra block, without replacing anything? I don't think this would ever be the case in MakeCode.

Yes! And I can make it happen in MakeCode too. This feature is called "healing a stack" or "splicing". It's implemented in unplugFromRow and unplugFromStack in core Blockly.

Examples

In the advanced Blockly playground, with our basic blocks:

And the same thing in MakeCode:

Explanation

Situations that trigger this:

• You have a parent block A with a single value input (print block)
  

• There is a connected block B with a single value output (number block: 3.14)
  

• There is a block C that has a single value input and a single value output, both of the appropriate type (square root block)
  

• Block C is dragged between blocks A and B

• When the drag ends, the blocks will be connected horizontally in order (from left to right) A, C, B
  

Note that in both cases there is a shadow block D that is initially attached to block C's input. Block D disappears, because block B covers it up.

Other info

You cannot trigger a splice if block C has two value inputs, because the correct operation is ambiguous:

Geras:

Zelos:

You can trigger a splice if block B has two value inputs, because the correct operation is unambiguous:

Geras:

Zelos:

[kmcnaught]

Ooh. That is what I had pictured in geras, but a bit surprising in MakeCode. Thanks for the examples!

My initial position is that we would expect the same behaviour from a keyboard-triggered "insert/replace" operation as you would get from a mouse drag. Now I understand more I can imagine that sometimes it's going to be a bit awkward if the splicing isn't what was intended. I don't think it changes my views on the core insert/replace flow, just a slightly surprising artefact of existing behaviour.

Do you have any intention to give more control over this, or does it change your views at all?

[rachel-fenichel]

I don't think it changes my views on the core insert/replace flow, just a slightly surprising artefact of existing behaviour.

Same here, especially because it lets us rely on existing (and well-tested) code in core, rather than throwing together a new layer of complex checks.

Please give insertion/replacement for inputs and outputs a try using the T key after #196 and let me know if it feels reasonable.

That change does not address the issues of when insert/replace should be in the context menu or what the text of the insert/replace option should be.

[kmcnaught]

I've just tried it. The general feel of inserting with 't' is very reasonable and aligned with what I was hoping for.

There's a slight weirdness that you end up with the cursor at the connection (which is usually skipped) after inserting. I don't know if this will lead to any bugs in the context menu etc, or if it's just a cosmetic bug.

I can imagine that once copy/paste is working, this will all feel quite powerful. It would be easy to e.g. remove the sin from the outside of a number block by copying/cutting the child, deleting the parent, and pasting the child in again.

I still don't quite understand the rules around splicing. There's a 30 second video here showing 2 situations where the splicing behaviour is different but I expected the same.

(to clarify - I don't think we should change splicing behaviour for keyboard nav, unless there are bug fixes that apply everywhere - I'm just getting my head around the logic)

[rachel-fenichel]

Yes, that behaviour is consistent with the other behaviour but is also confusing in the zelos case. If we change it for keyboard nav we may also want to change it for mouse. (In which case would file that as a separate bug to figure out the exact conditions for a different behaviour.)

Explanation:

Here's a gif of the same flow with two changes: I turned on the geras renderer and set the theme to the standard Blockly theme, which renders shadow blocks as lighter versions of real blocks, to make it easier to see the differences.

The behaviour is confusing because it relies on the distinction between shadow blocks and real blocks, which is minimal in zelos.

The x + y block is a real block, with two shadow children (1 and 4).

The sin block is a real block with one shadow child (45).

The repeat X times block is a real block with one shadow child (10).

Changing the value in the number block to 123 does not convert it to a real (non-shadow) block. There is a plugin that does convert shadow blocks to real blocks on edit.

When you insert the sin block at the output of the x + y block, it splices because the real x + y block is higher priority than the shadow number block (45) that was already attached to the sin block.

When you insert the sin block at the output of the 123 block it replaces because the shadow 123 is lower priority than the shadow 45 that was already attached to the sin block.

At the end of the gif I navigate to each sin block and delete it. Notice that the 123 comes back for the lower repeat: when a shadow block is obscured, its value is stashed and reapplied if the obscuring block is deleted. That's also why the 10 block reappears as a 10.

[kmcnaught]

Thanks for the explanation and demo, that makes a lot of sense. I can't help but favour the plugin's behaviour of turning a shadow block into a real block on edit - since that shows intent from the user to make something concrete. Are there situations or applications in which this wouldn't make sense, or previous discussions I can refer back to learn about the competing needs?

There are downstream consequences of this behaviour on copy and paste, which don't make up with my expectations. I would have assumed you'd be able to copy and paste the contents of an oval, like the text in a "write with shadow" block

I would naively argue that these blocks should be real, and draggable, and copyable. If they are not, then we definitely need some user feedback when they try to copy a block, because currently copy will silently fail and you'll end up surprised by pasting the last thing in the copy buffer.

[rachel-fenichel]

The main reason to have shadow blocks is that they are undeletable, so you can never end up with an empty input.

Converting it to a real block on change means that the edited block would get bumped out, instead of obscured, by dragging on a new block.

The issue with not being able to copy them is due to the precondition checks for copying and should be resolvable, so I've added an issue for that.

[rachel-fenichel]

Remaining work not required for testing, so this is now in the "simple editing complete" milestone.