-
Notifications
You must be signed in to change notification settings - Fork 31
Make blocks in Picker not editable #369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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
a356c9f
to
6e318f2
Compare
if not editable: | ||
return |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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…)
There was a problem hiding this comment.
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.
_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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could do this:
_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.
There was a problem hiding this 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.
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