pharo-spec / Spec

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

Fix MNU when using component list as input port in custom presenter #1463

Closed hernanmd closed 10 months ago

hernanmd commented 10 months ago

This PR handles a case with transmissions along a component list in custom presenters. There is an issue in transmitTo:transform: method when connecting a transmission between presenters where the target presenter has defined an input port as a SpListItemPort with a component list .

defaultInputPort 

    ^ SpListItemsPort newPresenter: componentListPresenter

The PR includes a case with a test to reproduce the problem:

SpComplexComponentListExample open.

To fix it, the PR specializes transmitTo:transform: to handle problem with connecting presenters with component list.

estebanlm commented 10 months ago

sorry but this fix is wrong.

estebanlm commented 10 months ago

transmissions do not work between presenters but between ports OutputPortA connects to InputPortB the method transmitTo: (and family) present in SpPresenter are convenience methods to connect the default output port of a presenter with the default input port of another presenter, in that sense, doing:

aPresenter transmitTo: otherPresenter

is the same as doing:

aPresenter defaultOutputPort transmitTo: otherPresenter defaultInputPort

but you cannot take a port and connect it to a presenter.

EDIT: basically, you cannot use a presenter as input port.

estebanlm commented 10 months ago

I made a revert:

https://github.com/pharo-spec/Spec/pull/1464

hernanmd commented 10 months ago

transmissions do not work between presenters but between ports OutputPortA connects to InputPortB the method transmitTo: (and family) present in SpPresenter are convenience methods to connect the default output port of a presenter with the default input port of another presenter, in that sense, doing:

aPresenter transmitTo: otherPresenter

is the same as doing:

aPresenter defaultOutputPort transmitTo: otherPresenter defaultInputPort

but you cannot take a port and connect it to a presenter.

EDIT: basically, you cannot use a presenter as input port.

The documentation isn't exactly clear about what's the difference between "using as" and "connecting".

The documentation mentions that "a transmission connects a presenter's output port with a presenter's input port," whereas you wrote, "transmissions do not work between presenters but between ports." This is contradictory because the documentation implies that the transmission connects presenters indirectly through their ports, whereas you suggest that the transmission works only between ports, not presenters.

Additionally you've mentioned "you cannot take a port and connect it to a presenter," which contradicts the information in the documentation, where it is shown that a port can be connected to a presenter's default input port, as in the example "list defaultOutputPort transmitTo: detail defaultInputPort".

I provided an connectPresenters use case where following the documentation does not work. What do you suggest to modify in order to run it without errors?

estebanlm commented 10 months ago

this part or your case:

defaultOutputPort 

    ^ self categoryListPresenter

is wrong. You cannot answer a presenter as a port.

demarey commented 10 months ago

There is a draft chapter on transmissions Using transmissions and ports in the Spec2 book: https://github.com/SquareBracketAssociates/BuildingApplicationWithSpec2/releases/download/latest/Spec2-wip.pdf

You can simply define #defaultOutputPort as follows:

defaultOutputPort 

    ^ self categoryListPresenter defaultOutputPort
hernanmd commented 10 months ago

There is a draft chapter on transmissions Using transmissions and ports in the Spec2 book: https://github.com/SquareBracketAssociates/BuildingApplicationWithSpec2/releases/download/latest/Spec2-wip.pdf

You can simply define #defaultOutputPort as follows:

defaultOutputPort 

    ^ self categoryListPresenter defaultOutputPort

Thanks @demarey ! That's exactly what I was looking for.