Block: Allow deleting with Delete button

[dbnicholson]

Allow a Block to be deleted with a confirmation dialog when the Delete
key is pressed. However, when the Block is in the picker (rather than
the canvas), it shouldn't be deleted. Add a flag to disable deleting and
restore it when a drag completes. Furthermore, when it shouldn't be
deleted, stop propagating the Delete key so it doesn't try to delete the
whole BlockCode node.

Fixes: #162

[dbnicholson]

@dylanmccall I'd appreciate if you could take a look at the mouse focusing since I'm not totally confident about how it interacts with the mouse_filter addition in e178d97.

Another thing that would be super useful (but I didn't attempt to tackle) is if the block could be drawn to show that it's focused. See the docs for a description. However, it's a bit difficult in our case since the thing we want to alter is the background node within the Block.

[manuq]

I don't think a confirmation is needed, since it's a bit cumbersome and its already easy to undo. For referece, MS Arcade has none.

Maybe deleting a block should not delete the ones stacked to its bottom?

Regarding focus, something is wrong in both main and this branch: when I scroll in the Output dock the Block Code canvas is zoomed in and out. As you can see in this video:
Grabación de pantalla desde 2024-07-29 14-44-24.webm

[dbnicholson]

I don't think a confirmation is needed, since it's a bit cumbersome and its already easy to undo. For referece, MS Arcade has none.

I guess I followed the scene editor where it asks for confirmation when deleting a node. Personally I find it helpful but I'm mostly indifferent.

Maybe deleting a block should not delete the ones stacked to its bottom?

I also was following the scene editor where it will delete the whole branch. Although the dialog does say "Delete ... and all it's children" in that case. Reparenting the lower blocks might be a little tricky. I'm also not sure if that could leave you with an invalid node tree. I'm inclined to delete the whole branch like the scene editor and leave any fancier work for a followup.

Regarding focus, something is wrong in both main and this branch: when I scroll in the Output dock the Block Code canvas is zoomed in and out. As you can see in this video: Grabación de pantalla desde 2024-07-29 14-44-24.webm

Oh, interesting. I also noticed that my block canvas was way zoomed in at some point but hadn't made the connection. I guess I need to explicitly [[ https://docs.godotengine.org/en/stable/classes/class_control.html#class-control-method-release-focus | release focus ]] when the canvas is no longer active.

[manuq]

I don't think a confirmation is needed, since it's a bit cumbersome and its already easy to undo. For referece, MS Arcade has none.

I guess I followed the scene editor where it asks for confirmation when deleting a node. Personally I find it helpful but I'm mostly indifferent.

Ok! I checked visual shader nodes and also don't have a confirmation. But I think we need learners feedback. So feel free to merge with the confirmation. Or maybe @cassidyjames or @jbourqueendless can tell us.
Grabación de pantalla desde 2024-07-30 16-22-46.webm

Maybe deleting a block should not delete the ones stacked to its bottom?

I also was following the scene editor where it will delete the whole branch. Although the dialog does say "Delete ... and all it's children" in that case. Reparenting the lower blocks might be a little tricky. I'm also not sure if that could leave you with an invalid node tree. I'm inclined to delete the whole branch like the scene editor and leave any fancier work for a followup.

Fair! I wasn't thinking them as children but as "stacked below". Like removing a line of code should not remove the ones below.

Regarding focus, something is wrong in both main and this branch: when I scroll in the Output dock the Block Code canvas is zoomed in and out. As you can see in this video: Grabación de pantalla desde 2024-07-29 14-44-24.webm

Oh, interesting. I also noticed that my block canvas was way zoomed in at some point but hadn't made the connection. I guess I need to explicitly [[ https://docs.godotengine.org/en/stable/classes/class_control.html#class-control-method-release-focus | release focus ]] when the canvas is no longer active.

👍

[dylanmccall]

I played with this a bunch today, and created #179 adding focus indicators. Feel free to merge it into this branch if it's easier to deal with :) It comes with the bonus that we almost have working keyboard navigation, although I'm very fed up with Godot's theming system at the moment. Need to sit down and try to understand it. Right now I'm doing a weird thing where I call add_theme_stylebox_override with a new "panel" stylebox when the focus changes, which doesn't seem right. It seems like there should be a "normal" and "focus" stylebox, but that isn't what's happening.

[dbnicholson]

Regarding focus, something is wrong in both main and this branch: when I scroll in the Output dock the Block Code canvas is zoomed in and out. As you can see in this video: [Grabación de pantalla desde 2024-07-29 14-44-24.webm]

I can't reproduce this anymore either in main, the branch here (based on an older main), or a rebased version of this branch. What I started looking at was disabling input event handling for the block canvas when the main panel is hidden:

diff --git a/addons/block_code/ui/main_panel.gd b/addons/block_code/ui/main_panel.gd
index ce628ce..39504ed 100644
--- a/addons/block_code/ui/main_panel.gd
+++ b/addons/block_code/ui/main_panel.gd
@@ -47,6 +47,8 @@ func _ready():
        if not _collapse_button.icon:
                _collapse_button.icon = _icon_collapse
 
+       visibility_changed.connect(_on_visibility_changed)
+
 
 func _on_undo_redo_version_changed():
        if _current_block_code_node == null:
@@ -79,6 +81,11 @@ func _on_delete_node_button_pressed():
        pass  # Replace with function body.
 
 
+func _on_visibility_changed():
+       print("Main panel visibility changed to %s" % visible)
+       _block_canvas.set_process_input(visible)
+
+
 func _on_delete_dialog_confirmed(block_code_node: BlockCode):
        var parent_node = block_code_node.get_parent()
 

Since I couldn't reproduce it anymore, I decided to leave that out.

[dbnicholson]

Ready for review again. It still has the confirmation dialog and still deletes the entire tree of blocks below it. What's changed:

• I pulled in Add focus indicator for blocks #179 with a minor change.
• I went with @wnbaum's suggestion to use a simpler Delete <N> blocks? prompt like in Scratch. I really wanted to put a useful name there but we don't have anything at the moment and I didn't want to go making a bunch of changes in CategoryFactory when we're about to redo that whole thing.

[dylanmccall]

I gave this another look and I'm happy with where it is. I was able to reproduce that issue with scrolling in other bottom panel tabs, but since it isn't a regression in this branch, I reported it separately in #184. I rebased this branch against main, and I'll merge it in a moment :)