Finish chapter "Managing windows"

[koendehondt]

• [x] Check all code examples and images

Open issues from https://github.com/SquareBracketAssociates/BuildingApplicationWithSpec2/pull/50:

Section on setting the icon of a window

• [x] The explanation and the example are wrong. Sending windowIcon: has no effect. SpWindowPresenter>>#taskbarIcon is deprecated (it is in the TOREMOVE protocol). Setting the icon is not implemented in Spec 2. I think this incompleteness should be fixed in Pharo 12, otherwise this section has to be removed.

Section on setting the about text

• [x] SpWindowPresenter>>#aboutText: is in the TOREMOVE protocol, so this section is not up-to-date. Similar to the section on the icon, I think this issue should be addressed in Pharo 12.

[koendehondt]

@Ducasse We have a problem with the explanation in section "Preventing window close". That section states that implementing:

WindowExamplePresenter >> okToChange

   ^ false

makes the window uncloseable. That is not correct. requestWindowClose has to be implemented instead. That is clear fromSystemWindow>>#closeBoxHit:

closeBoxHit
    "The user clicked on the close-box control in the window title.
    For Mac users only, the Mac convention of option-click-on-close-box is obeyed if the mac option key is down.
    If we have a modal child then don't delete.
    Play the close sound now since this is the only time we know that the close is user-initiated."

    self allowedToClose ifFalse: [^self].
    self playCloseSound.
    self close

and SpWindow>>#allowedToClose:

allowedToClose

    super allowedToClose ifFalse: [ ^ false ].
    self model ifNil: [ ^ true ].
    ^ self model askOkToClose 
        ifTrue: [ self model requestWindowClose ]
        ifFalse: [ true ].

The latter sends requestWindowClose, but its implementationSpPresenter>>#requestWindowClose is categorized in the TOREMOVE protocol:

requestWindowClose

    "returns <true> if the user is allowed to close the window. Useful if you want to ask user if he wants to save the changed content etc."

    ^ true

So it is unclear to me whether using requestWindowClose is a good idea. Is the protocol name incorrect? Or are there plans with Spec that I am not aware of?

Until we have resolved this issue, I will put this in the book because that is needed to support the explanation in this chapter.

WindowExamplePresenter >> requestWindowClose

   ^ false

[koendehondt]

Forgot to add: there are no senders of requestWindowClose in the image. Strange.

[koendehondt]

@Ducasse Another issue. The same section uses self askOkToClose: true, which is implemented as:

askOkToClose: aBoolean

    askOkToClose := aBoolean

but the implementation of the accessor SpPresenter>>#askOkToClose is:

askOkToClose
    "DO NOT USE
    With Spec 2, SpPresenter was refactored to move all window management to WindowPresenter.
    From now on, if you want to interact with a window you need to:
    - Implement #initializeWindow: method (#initializeDialog: for dialogs) to manage window elements before the presenter is opened
    - Use the method #window or #withWindowDo: to interact with windows after it has been opened.

    This method cannot be deprecated because during a transition phase we keep this mecanism. "

    ^ askOkToClose

There is something fishy about all the window closing behavior.

[Ducasse]

I have no idea :( We should ask @estebanlm else we remove it from the book because I do not want to document something that will be removed.

[estebanlm]

yeah, do not use it :)

[estebanlm]

there is an event (whenWillCloseDo:) to achieve the same now). it reaceives an event that can denyClose if needed.

[koendehondt]

About

SpWindowPresenter>>#aboutText: is in the TOREMOVE protocol, so this section is not up-to-date. Similar to the section on the icon, I think this issue should be addressed in Pharo 12.

I think SpWindowPresenter>>#aboutText: is in the TOREMOVE protocol by mistake because there is no other way to set the about text of a window. @estebanlm could you explain why the method is in the TOREMOVE protocol? Are there plans with the about text that are not documented in the code?

[koendehondt]

About

The explanation and the example are wrong. Sending windowIcon: has no effect. SpWindowPresenter>>#taskbarIcon is deprecated (it is in the TOREMOVE protocol). Setting the icon is not implemented in Spec 2. I think this incompleteness should be fixed in Pharo 12, otherwise this section has to be removed.

After careful inspection of the code, I conclude that setting the taskbar icon for a window is not implemented correctly. Look at SpPresenter>>#initializeWindow::

initializeWindow: aWindowPresenter
    "override this to set window values before opening. 
     You may want to add a menu, a toolbar or a statusbar"

    "IMPORTANT: Please ovirride this method and set yourself the informations you want in your window.
    The content of this method is here to help the transition between Spec 1 and 2.
    In the next Spec version the content of this method will be removed and it will do nothing by default because the goal is to remove the management of all of those informations from Composable to put them in WindowPresenter."

    aWindowPresenter
        title: self title;
        initialExtent: self initialExtent;
        windowIcon: self windowIcon

As stated before, sending windowIcon: self windowIcon does not have an effect. The method is not used anymore.

SystemWindow>>#taskbarTask does this:

taskbarTask
    "Answer a taskbar task for the receiver.
    Answer nil if not required."

    (self valueOfProperty: #noTaskbarTask ifAbsent: [false]) ifTrue: [^nil].
    taskbarTask := TaskbarTask
        morph: self
        state: self taskbarState
        icon: (self iconNamed: self taskbarIconName)
        label: self taskbarLabel.
    ^taskbarTask

and taskbarIconName does this:

taskbarIconName
    "Answer the icon for the receiver in a task bar."

    self model ifNotNil: [
        self model taskbarIconName
            ifNotNil: [ :aName | ^aName ] ].

    ^ super taskbarIconName

Which means that the model of a window should respond to taskbarIconName. In Spec applications, the model is a SpWindowPresenter. That class does not implement taskbarIconName. It inherits it from Object:

taskbarIconName
    "Answer the icon for the receiver in a task bar
    or nil for the default."

    ^self class taskbarIconName

and Object class implements:

taskbarIconName
    "Answer the icon for an instance of the receiver in a task bar"

    ^#smallWindow

Due to this implementation, all Spec windows in the task bar are shown with the #smallWindow icon. And there is no way to change it!

Here is an example that does not work as expected. StPlaygroundPresenter has this method to specify the task bar icon:

windowIcon

    ^ self application iconNamed: #workspace

but that method is never invoked, and therefore playground windows show up like this in the task bar:

This icon should be shown according to the intentions of the StPlaygroundPresenter class:

The consequence of this wrong behavior is that the section Setting the icon of the chapter Managing windows cannot be included. So @Ducasse I propose to take it out.

I will also create an issue for the wrong behaviour. We have to fix that. Actually there is a lot of related code that is not, or should not, be used anymore. We have to clean that up and make sure that Spec application developers (like me 😁) can specify which icon to show in the task bar.

[koendehondt]

@Ducasse and @estebanlm As announced in a previous comment, here is the issue to describe the problem and to suggest a solution: https://github.com/pharo-spec/Spec/issues/1546.

[Ducasse]

Let us take it out (Setting the icon of the chapter Managing windows)

[koendehondt]

Let us take it out (Setting the icon of the chapter Managing windows)

Consider it done.

[koendehondt]

@estebanlm @Ducasse

there is an event (whenWillCloseDo:) to achieve the same now). it reaceives an event that can denyClose if needed.

Sadly there is a bug that does not allow me to write a decent example in the chapter: https://github.com/pharo-spec/Spec/issues/1547