Skip to content

Conversation

manuq
Copy link
Contributor

@manuq manuq commented Jan 29, 2025

Add a new "editable" boolean property to Block, TemplateEditor and ParameterInput. When the block is in the picker, it will not be editable. Only when they are dragged to the canvas they become editable.

The property has to be in each of these components because Block can't access its ParameterInputs directly, only through its TemplateEditor.

The dropdown options in a not-editable block can still be changed to avoid disabling the button. But the change doesn't trigger any effect.

The default is editable = true, so blocks in the canvas are editable.

Fix #357

Add a new "editable" boolean property to Block, TemplateEditor and
ParameterInput. When the block is in the picker, it will not be
editable. Only when they are dragged to the canvas they become editable.

The property has to be in each of these components because Block can't
access its ParameterInputs directly, only through its TemplateEditor.

The dropdown options in a not-editable block can still be changed to
avoid disabling the button. But the change doesn't trigger any effect.

The default is editable = true, so blocks in the canvas are editable.

Fix #357
@manuq manuq force-pushed the picker-not-editable branch from a356c9f to 6e318f2 Compare January 29, 2025 11:37
Comment on lines +199 to +200
if not editable:
return
Copy link
Member

Choose a reason for hiding this comment

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

It seems weird to me that this is necessary when _line_edit.editable is set to match editable in _set_editable. I can see that if I focus (e.g.) the text field in the "log text" block, I can press Enter and trigger the callback that calls this function. But I can't find any way to actually change the value, so the check below also makes the function return early.

I suppose it's harmless to have this, and maybe there's some case I've overlooked which means it's actually necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wjt you are right, this isn't necessary at all. I added this early return to avoid the emition of the modified signal at the end of this function. Nothing should be connected to that signal anyways for blocks in the picker. So do you think is better to remove the early returns from this patch?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's okay to keep it, I was just wondering what it's here for.

Copy link
Member

Choose a reason for hiding this comment

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

(I think it is weird that signals like LineEdit.text_submitted can be emitted even when the field is not editable…)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I think the same.

Comment on lines +68 to +73
_line_edit.editable = value
_x_line_edit.editable = value
_y_line_edit.editable = value
_v3_x_line_edit.editable = value
_v3_y_line_edit.editable = value
_v3_z_line_edit.editable = value
Copy link
Member

Choose a reason for hiding this comment

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

You could do this:

Suggested change
_line_edit.editable = value
_x_line_edit.editable = value
_y_line_edit.editable = value
_v3_x_line_edit.editable = value
_v3_y_line_edit.editable = value
_v3_z_line_edit.editable = value
for line_edit in _last_submitted_text:
line_edit.editable = value

though I don't know if it's an accident that these 6 controls happen to match that dictionary.

Copy link
Member

@wjt wjt left a comment

Choose a reason for hiding this comment

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

This seems fine to me – feel free to address or leave the comments above.

@manuq manuq merged commit 19eb9f6 into main Jan 29, 2025
3 checks passed
@manuq manuq deleted the picker-not-editable branch January 29, 2025 14:52
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.

Block Picker: Text input should not be editable

2 participants