Adapt contextmenu to actions #119
Conversation
|
Thanks esteban. |
|
also, code is not yet integrated into the image so... better to wait a bit. But I wanted to start fixing this otherwise I will forget it later ;) |
|
Oki |
|
@estebanlm I see that you introduced commands in chapters before the chapter on Commander. Using concepts before they are introduced is not a good idea. I propose to keep the menus in the chapters before the Commander chapter and use commands only in the Commander chapter. |
|
thing is, there is not anymore just menus, they are now handled through actions (which are, indeed, "generic" commands). you can introduce actions as simple definitions, that define both "visible" actions which will be displayed as context menus and "not visible" actions that will be accessible through shortcuts. |
Could you explain what you mean with "now" in " they are now handled through actions"? Do you refer to a particular Pharo version? |
| @@ -147,16 +147,19 @@ Look at the shortcuts in the `messageMenu` method. `$n meta` means that the char | |||
|
|
|||
| ### Installing shortcuts | |||
|
|
|||
| Adding shortcuts to menu items does not automatically install them. Keyboard shortcuts have to be installed after the window has been opened. Therefore we have to adapt the `initializeWindow:` method with the `whenOpenedDo:` message, so that the keyboard shortcuts can be installed after opening the window. `SpMenuPresenter`, which is the superclass of `SpMenuBarPresenter`, implements the method `addKeybindingsTo:`, which comes in handy here. | |||
| Adding shortcuts to menu items does not automatically install them. Keyboard shortcuts have to be installed after the window has been opened. Therefore we have to adapt the `initializeWindow:` method with the `whenOpenedDo:` message, so that the keyboard shortcuts can be installed after opening the window. `SpMenuPresenter`, which is the superclass of `SpMenuBarPresenter`, implements the method `addKeyBindingsTo:`, which comes in handy here. | |||
There was a problem hiding this comment.
This suggestion is not correct. SpMenuPresenter>>#addKeybindingsTo: is written with a lowercase b, contrary to other method selectors with "KeyBinding" in them, like SpAbstractMorphicAdapter>>#addKeyBindingsTo:.
|
|
||
| "this will copy the menubar shortcuts from menuBar to the window presenter, | ||
| to allow the window to answer to them" | ||
| menuBar addKeyBindingsTo: aWindowPresenter |
There was a problem hiding this comment.
| menuBar addKeyBindingsTo: aWindowPresenter | |
| menuBar addKeybindingsTo: aWindowPresenter |
| toolbar: toolBar | ||
| toolbar: toolBar. | ||
|
|
||
| menuBar addKeyBindingsTo: aWindowPresenter |
There was a problem hiding this comment.
| menuBar addKeyBindingsTo: aWindowPresenter | |
| menuBar addKeybindingsTo: aWindowPresenter |
| statusBar: statusBar | ||
| statusBar: statusBar. | ||
|
|
||
| menuBar addKeyBindingsTo: aWindowPresenter |
There was a problem hiding this comment.
| menuBar addKeyBindingsTo: aWindowPresenter | |
| menuBar addKeybindingsTo: aWindowPresenter |
|
|
||
| The tree includes folders and emails, so the desired menu items should be disabled when a folder is selected. They should also be disabled when no selection has been made. On top of that condition, the send command can only be applied to emails that are in the "Draft" folder because received and sent mails cannot be sent. | ||
|
|
||
| Typically, a presenter adds a context menu to a subpresenter. Given that the tree of folders and emails is a subpresenter of the `MailAccountPresenter`, we would expect the `MailAccountPresenter` to install a context menu on the tree presenter. However, the `MailAccountPresenter` cannot decide what needs to be done for deleting or sending an email. What needs to be done is the responsibility of the `MailClientPresenter`, which defines the methods `deleteMail` and `sendMail`. Both methods do what they have to do to perform the action, and then send the `modelChanged` message and update the status bar. | ||
|
|
||
| Therefore `MailClientPresenter` defines the menu. | ||
| Therefore `MailClientPresenter` defines the context menu for the subpresenter, creating an *action grouop*. |
There was a problem hiding this comment.
| Therefore `MailClientPresenter` defines the context menu for the subpresenter, creating an *action grouop*. | |
| Therefore `MailClientPresenter` defines the context menu for the subpresenter, creating an *action group*. |
|
@estebanlm, @Ducasse and I discussed this PR. We will not merge it because it requires Pharo 13 and the book uses Pharo 12. I applied this change suggested by this PR: SquareBracketAssociates/CodeOfSpec20Book@ea88929. Let's not delete this PR so that it can serve as the basis for the book upgrade to Pharo 13, sometime during the year 2025. |
this PR adapts the examples and explanations to the new action architecture, that will be used to add context menus and keybindings to presenters.