Allow copy/select shortcuts when editable==false in LineEdit
#99822
Conversation
scene/gui/line_edit.cpp
Outdated
| accept_event(); | ||
| return; | ||
| } | ||
|
|
||
| // Cut / Paste | ||
| if (k->is_action("ui_cut", true)) { |
There was a problem hiding this comment.
Cut should copy when not editable, so it matches with TextEdit behavior.
The cut_text() method can modified for this.
There was a problem hiding this comment.
IMO the TextEdit's behavior is wrong.
For reference, Cut doing nothing in readonly fields is how browsers behave:
https://www.w3schools.com/tags/tryit.asp?filename=tryhtml_input_readonly
There was a problem hiding this comment.
The text in your example gets copied on Windows / Firefox using Ctrl+x.
There was a problem hiding this comment.
Yes, that's Copy. Ctrl+X does nothing.
There was a problem hiding this comment.
Sorry, I meant using Ctrl+X
There was a problem hiding this comment.
Indeed, but it does not on Chrome 🤔
There was a problem hiding this comment.
It also does not on MS Edge
havi05
force-pushed
the
lineedit-shortcuts
branch
2 times, most recently
from
21aa684
to
a6bdbed
Compare
|
What do you think about merging the |
scene/gui/line_edit.cpp
Outdated
| if (editable && selection.enabled && !pass) { | ||
| if (selection.enabled && !pass) { | ||
| DisplayServer::get_singleton()->clipboard_set(get_selected_text()); | ||
| selection_delete(); | ||
| if (editable) { | ||
| selection_delete(); | ||
| } |
There was a problem hiding this comment.
This is wrong. Cutting should always be a complete operation.
There was a problem hiding this comment.
I'll change it to use copy_text instead and revert the change of the function.
There was a problem hiding this comment.
No, the Cut shortcut just should do nothing if LineEdit is not editable.
There was a problem hiding this comment.
Should the behavior of TextEdit be changed to match this one? (In a seperate pull request)
There was a problem hiding this comment.
Yes.
Also we could add tests for these fixes.
There was a problem hiding this comment.
tbh this can be discussed further.
I think cut not actually cutting text, but only copying it, is wrong. Cut is an operation that removes text and copies it to clipboard, so it should not be doing one of them. As I noted #99822 (comment), that's how readonly fields behave in browsers, which is a nice reference for how things should work.
That said, TextEdit even has text for this behavior, so maybe it's better to open an issue about it. Currently the Cut operation is disabled in menu, so it's weird that it would do something via shortcut.
(I'll unresolve the comment, so this discussion is visible)
Makes sense, but not in this pull request. |
havi05
force-pushed
the
lineedit-shortcuts
branch
from
a6bdbed
to
f377367
Compare
|
Since this is disputable, and currently inconsistent with TextEdit, I think you could change the EDIT: |
havi05
force-pushed
the
lineedit-shortcuts
branch
from
f377367
to
47ce8d0
Compare
|
I've removed the comment |
This fixes #99813
I just moved the
context_menu,copyandselect_allinput checks before theeditablecheck in line 651.