Hi Christoph --
I added the missing checks via Morphic-mt.2126 ToolBuilder-Morphic-mt.346
Still, the current design is not great in that - the return values key-handling methods are inconsistent between PluggableList and PluggableTree - several over-cautious (or too early) #okToChange sends for things like pop-up menus
as you observed. :-)
Best, Marcel Am 28.08.2023 16:57:34 schrieb Marcel Taeumel marcel.taeumel@hpi.de: Hi Christoph --
Sending #okToChange from the widgets seems to be bad style to me
Yet, this is what PluggableListMorphs do at the moment. I think it makes sense to extend PluggableTreeMorph in a similar way. Otherwise, the TreeBrowser being an extension of Browser makes no sense anymore.
Best, Marcel Am 26.08.2023 17:13:11 schrieb christoph.thiede@student.hpi.uni-potsdam.de christoph.thiede@student.hpi.uni-potsdam.de: Hi Marcel, hi all,
here is a couple of simple way to reproduce:
Browse a class in the tree browser. Select message category A. Start typing a method but do not yet accept it ('myMethod ^self greatNewInvention...') Turn on mouseOverForKeyboardFocus, hover the message category list, press arrow up/down to select a different category. Oh no! Your new unaccepted method is lost!
Other triggers of this bug include removing the currently selected message category in a tree browser from a second browser (or adding the first category which will remoev the default 'no messages' category).
Quickfix to recover your changes (maybe):
String allSubInstances select: [:ea | ea includesSubstring: 'greatNewInvention'].
Insights from my examination:
There is a missing #okToChange in Browser>>selectMessageCategoryNamed:. The bug does not occur when selecting a different category through the mouse because PluggableListMorph and SimpleHierarchicalListMorph send #okToChange to the model before handling mouse click events. PluggableListMorph also sends this for keyboard events (such as keyDown), but SimpleHierarchicalListMorph doesn't. When the selection is triggered programmatically, e.g., through #updateCodePaneIfNeeded, neither sends #okToChange first.
So, how can we fix this? I think we should be sure to send #okToChange prior to all selection changes in browsers. We should not rely on the widget to send this in any case, flag existing uses with a comment, and maybe ultimately remove these sends. Sending #okToChange from the widgets seems to be bad style to me - it is hard to discover, existing triggers such as user interactions vs programmatic events are disputable, and importantly, this style introduces global state. I think I have also seen more than one model in the past where I was spammed with irrelevant 'Is it OK to cancel those changes?' messages because the actual interaction would not actually cancel any changes, but I unfortunately can't find these right now.
Best, Christoph
--- Sent from Squeak Inbox Talk [https://github.com/hpi-swa-lab/squeak-inbox-talk]