Quentin Mathé wrote: > Hi Fred, > > Le 9 mars 08 à 16:28, Fred Kiefer a écrit : > >> Putting it into the bug tracker wont help as long as nobody takes up the >> task to solve the issue :-) >> Which is what I did myself already. I provided a workaround in the >> toolbar code right after reporting the issue. What I wanted to prevent >> was Adam doing a release with this problem not addressed. >> In general you are of course correct, we should put all the issues we >> find in the bug tracker, even if we are the ones o work on them later. I >> already have taken up this habit for all more complex issues. >> >> My solution is currently only a hack. This is due to my missing >> understanding of the NSToolbar related code. To me it is even unclear, >> why there is GSToolbar and NSToolbar. > > When I wrote NSToolbar implementation, I introduced GSToolbar superclass > in order to allow inserting a toolbar anywhere in a window and not just > underneath the title bar as NSToolbar only supports it. > If you run ToolbarExample from gui examples, you can see a demo of this > feature. May be I stretched the API a bit too far and we should either > remove this feature or merge NSToolbar and GSToolbar into a single > class. GSToolbar doesn't allow to change display and size modes once it > has been initialized. That's the main difference with NSToolbar that > mainly adds methods --setDisplayMode:, -setSizeMode: which alters the > toolbar view frame. This resizing can easily be handled automatically in > NSToolbar case because the toolbar is always located at the top of the > window. > >> As for GSToolbarButton, I have the >> impression that this class uses an ivar to store the action, instead of >> passing it to the cell as would be the normal case, because of the >> recursion to bumped into. With that properly resolved it looks like we >> could clean up more of the NSToolbar cluster. But is my hack a proper >> solution? I am not sure. While investigating the code I found a second >> recursion, also stopped by my patch. > > I would need to take a deeper look at it, but iirc I introduced these > not-so-clean workarounds because of issues I encountered with actions > and -mouseDown: when using NSButton and NSButtonCell three years ago. > Toolbar buttons have to be highlighted on mouse down and send the action > on mouse up. If they are selectable (like tabs), the same behavior must > occur on -[NSToobar setSelectedItemIdentifier:] but the highlighting > must be retained until another selectable item in the toolbar is clicked > or or selected with the previous method in code. The code can probably > be rewritten in a more natural way today. > >> To me this looks like the tasks are not clearly separated between the >> different components. But then I am not sure, there may be some code out >> there, that relies on any of the strange looking features here. > > -_setSelected: shouldn't invoke -performClick: probably but rather just > changes the state of the toolbar button to be highlighted, redraws it > and sends the action. Calling -performClick: was convenient because > -_setSelected: must do precisely what happens when the user clicks a > toolbar item that is selectable. I think that was my reasoning at the > time I wrote this code. > Another point is that it could be more correct or cleaner to handle most > of -_setSelected: logic in -[GS/NSToolbar setSelectedItemIdentifier:]. >
Thank you for this long explanation. I will keep it in case I do more changes on NSToolbar. Could you please test, whether a tool bar after my change still behaves as expected in all relevant cases? I tried to run the example code your pointed me to (Thank you for that as well!), but this horribly fails with issues that seem unrelated to my change. _______________________________________________ Gnustep-dev mailing list [email protected] http://lists.gnu.org/mailman/listinfo/gnustep-dev
