pharo-spec / Spec

Spec is a framework in Pharo for describing user interfaces.
MIT License
62 stars 65 forks source link

Bug: Menu item enablement in CmUICommandDisplayStrategy>>#display:in:do: is fixed #1603

Closed koendehondt closed 1 month ago

koendehondt commented 2 months ago

Context : Pharo 12 and Pharo 13.

I think CmUICommandDisplayStrategy>>#display:in:do: is implemented incorrectly. Now it is:

display: aCmSpecCommand in: aMenuOrGroupPresenter do: aBlock
    aMenuOrGroupPresenter
        addItem: [ :item |
            aBlock value: item.
            item enabled: aCmSpecCommand canBeExecuted.
            item ]

As a consequence, a menu item's enablement state is set only once, when the menu item is added. Given that the command decides whether it can be executed, the enablement state should be updated every time a menu item is displayed. In Spec, that is achieved by passing a block to #enabled:.

So I think the method should be implemented as:

display: aCmSpecCommand in: aMenuOrGroupPresenter do: aBlock
    aMenuOrGroupPresenter
        addItem: [ :item |
            aBlock value: item.
            item enabled: [ aCmSpecCommand canBeExecuted ].
            item ]

This issue is related to https://github.com/pharo-spec/Spec/issues/1559 and https://github.com/pharo-spec/Spec/issues/1561, which report other enablement issues.