Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question on SpBoxLayout>>#addLast: #1614

Closed
koendehondt opened this issue Oct 4, 2024 · 10 comments
Closed

Question on SpBoxLayout>>#addLast: #1614

koendehondt opened this issue Oct 4, 2024 · 10 comments

Comments

@koendehondt
Copy link
Contributor

koendehondt commented Oct 4, 2024

In Pharo 12 and Pharo 13, the implementation of SpBoxLayout>>#addLast: is:

addLast: aName
	"Adds `aPresenterLayoutOrSymbol` to the **end of the list** of presenters to be arranged 
	 in the layout. 
	 `aPresenterLayoutOrSymboll` can be
		- any instance of `SpPresenter` hierarchy, 
		- another layout or 
		- a Symbol, matching the name of the instance variable who will contain the element to add."

	self flag: #doNotUse.

	self 
		addLast: aName 
		withConstraints: [ :constraints | ]

Why is self flag: #doNotUse. here? It would be good to have an explanation in the method. What is the alternative?
There are many senders of the message, so why are they still present if this method should not be used anymore?

Also note that the name of the method argument (aName) does not correspond with the name used in the method comment (aPresenterLayoutOrSymbol).

@koendehondt
Copy link
Contributor Author

The method has been changed several times in the past year. self flag: #doNotUse. has been added and removed several times. So is the flag still valid?

Screenshot 2024-10-04 at 08 10 41

@koendehondt
Copy link
Contributor Author

Also see #1615

@Ducasse
Copy link
Contributor

Ducasse commented Oct 4, 2024

@estebanlm what is the replacement for this message? Because we must update the book because we use it.

@estebanlm
Copy link
Member

Hi,

you have several (and keeping addLast: imposes some problems on backend implementation, particularly on morphic).

You can do:

SpBoxLayout newLeftToRight
  add: aPresenter; "this presenter will be expanded"
  add: otherPresenter expand: false; "this one will not, making it 'last'"
  yourself

or you can do:

SpBoxLayout newLeftToRight
  hAlignEnd;
  add: otherPresenter;  "this presenter will be placed at the 'end' of the box."
  yourself

@estebanlm
Copy link
Member

@koendehondt was asking why?' : well, because I implemented that before implementing the hAlignStart/Center/End idioms and it looked like a good idea at first. Then I implemented it and suddenly I have 3 ways of doing the same (and 3 things to maintain) so I decided to slowly deprecate the less flexible way ( slowly: that's why is marked as #doNotUse and not deprecated directly ;) )

Can we close this issue now?

@koendehondt
Copy link
Contributor Author

@estebanlm In case other people read this ticket, I like to add a comment.

The examples that you gave are not inline replacements for the things you can do with addLast:.

This code applies a box layout that puts a button on the left and on the right:

SpPresenter new
	layout: (SpBoxLayout newLeftToRight
		add: (SpButtonPresenter new label: 'Button 1'; yourself) expand: false;
		addLast: (SpButtonPresenter new label: 'Button 2'; yourself) expand: false;
		yourself);
	open
Screenshot 2024-10-04 at 12 28 44

This cannot be achieved with the examples you gave. Nesting presenters is required, like this:

left := SpPresenter new.
left layout: (SpBoxLayout newLeftToRight
	add: (SpButtonPresenter new label: 'Button 1'; yourself) expand: false;
	yourself).
right := SpPresenter new.
right layout: (SpBoxLayout newLeftToRight
	hAlignEnd;
	add: (SpButtonPresenter new label: 'Button 2'; yourself) expand: false;
	yourself).
SpPresenter new
	layout: (SpBoxLayout newLeftToRight
		add: left;
		add: right;
		yourself);
	open

It opens a window in the same way as the previous code:

Screenshot 2024-10-04 at 12 31 44

I will adapt the text of the book accordingly. In particuar, this piece of text in Chapter "Layouts" has to be replaced, and the accompanying code in the code repo has to be adapted:

The defaultLayout method adds button button3 to the box layout with addLast:expand:fill:padding:. For every method starting with add:, the SpBoxLayout class provides a similar method starting with addLast:.

A box layout has two parts: a "start" and an "end". Messages starting with add:, add subpresenters to the "start". Messages starting with addLast:, add subpresenters to the "end". As you can see in Figure @ThreeButtons@, there is a large gap between button2 and button3. For vertical box layouts, the "start" part of a box layout aligns to the top side of the box. The "end" part aligns to the bottom side of the box. The gap between the "start" and the "end" is all the excess space not used by the subpresenters.

@koendehondt
Copy link
Contributor Author

As to your question "Can we close this issue now?", I would say yes, although it would be better to add a ticket for deprecating the addLast:... methods and cleaning up the senders in the Pharo tools. Otherwise other people will ask the same question I asked here. After all, just writing self flag: #doNotUse does not convey any helpful information to the reader.

@estebanlm
Copy link
Member

the behavior you ask for is still covered by the other, more flexible functionality, this script will cover what you want in a more elegant way than your example (yes, by nesting boxes, and aligning them correctly) :

(p := SpPresenter new)
	layout: (SpBoxLayout newLeftToRight
		add: (p newButton label: 'Button 1'; yourself) expand: false;
		add: (SpBoxLayout newLeftToRight
			hAlignEnd;
			add: (p newButton label: 'Button 2'; yourself);
			yourself)	;
		yourself);
	open.

@estebanlm
Copy link
Member

added an issue: #1616

@koendehondt
Copy link
Contributor Author

Thank you for the answers and the new ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants