VocabBuilder.koplugin: supports removing word in-place after addition#14127
Conversation
| if button then | ||
| button:disable() | ||
| if not button then return end | ||
| if button.text == _("Add to vocabulary builder") then |
There was a problem hiding this comment.
That seems a tad unexpected as a condition compared to some other mode or property?
There was a problem hiding this comment.
I thought it was the obvious way without adding another property. Should I add an ad-hoc property to the button like is_remove instead?
There was a problem hiding this comment.
A property or local variable makes the most sense to me, yes. I wouldn't expect a text = "something" assignment to be used like this.
| return "ui", button.dimen | ||
| end) | ||
| else | ||
| UIManager:show(ConfirmBox:new{ |
There was a problem hiding this comment.
is a confirmBox really necessary here? only asking if it wouldn't be better to directly undo without fuss
There was a problem hiding this comment.
Just in case that a word is not accidentally removed, especially when the word was added before with reviewing data. Since this button is rarely used, a little inconvenience should not be too much of a problem.
There was a problem hiding this comment.
I was just trying this, and if a word was already in the vocab builder, if you try to add it again (by accident or not), it will prompt you to decide if you'd like to overwrite the existing data, so maybe we don't need the confimrbox after all?
There was a problem hiding this comment.
One might accidentally press the button twice during a lookup and scroll.
There was a problem hiding this comment.
that seems quite unlikely, or at least far less likely than simply adding one by accident, just mentioning it though, to see if anyone else has an opinion.
This closes #14123;
This change is