Selection bug when checkbox in list

[Nyan11]

The labels do not displayed correctly when toggle buttons are in list.

Reproduce

group := ToCheckableGroup new.
moveListElement := ToListElement new.
moveListElement selectionMode selectOnMouseDown: false.
moveListElement nodeBuilder: [ :node :move :holder |
	| moveElement moveNumberLabel whiteMoveLabel moveButtons blackMoveLabel |
	moveButtons := OrderedCollection new.
	moveElement := BlElement new.
	moveElement layout: BlLinearLayout horizontal.
	moveElement margin: (BlInsets all: 5).
	moveElement constraintsDo: [ :c |
		c vertical fitContent.
		c horizontal fitContent ].

	moveNumberLabel := BlTextElement new.
	moveNumberLabel text: move first asString asRopedText.
	moveNumberLabel constraintsDo: [ :c | 
		c linear vertical alignCenter.
		c horizontal exact: 30 ].

	whiteMoveLabel := ToToggleButton new.
	whiteMoveLabel size: 30 @ 20.
	whiteMoveLabel labelText: move second asString asRopedText.
	moveButtons add: whiteMoveLabel.

	blackMoveLabel := ToToggleButton new.
	blackMoveLabel size: 30 @ 20.
	blackMoveLabel labelText: move third asString asRopedText.
	moveButtons add: blackMoveLabel.
	blackMoveLabel addStamp: #link.

	moveElement addChild: moveNumberLabel.
	moveElement addChild: whiteMoveLabel.
	moveElement addChild: blackMoveLabel.

	moveNumberLabel constraintsDo: [ :c | c ignored vertical alignCenter ].
	
	moveButtons do: [ :e | group register: e ].
	node addChild: moveElement
].
moveListElement size: 800 @ 600.
moveListElement dataAccessor addAll: { { 1. 'e4' . 'e5'} . {2. nil . nil} }.

space := BlSpace new.
space root addChild: moveListElement.
space pulse.
space resizable: true.
space show

Execute the code in playground.
Click on all toggle buttons and select a line in the list.

[Nyan11]

In Toplo, the space assigned states to each elements.
The state determine if a skin need to be installed, when an element is hover or pressed, or when an element is selected or unselected.
Then each element can see if it has a new state and if so, the element can changed its look (visual properties) based on its skin.

The bug is trigger because the ToLabel element of the toggle button receive a ToSelectionState caused by the ToListElement.
So when the label apply a "selection state" it's color change.

To debug it you can place two debug points

• In the method ToSkinStateGenerator >> #generateSelectionStateFromEvent: | it will stop before the ToSelectionState is apply to all selected elements of the list.
• In the method ToLabelSkin >> #selectedSkinEvent: | where the state is "applied" and the labels changes. (Be careful it can open a lot of debugger).

It is a Toplo bug.

• To fix it, the skin of the label would need to know if it is a simple label present in a list or if it a label present in a button + all other strange cases you could encounter.

OR

• We could change the method #generateSelectionStateFromEvent: to not affect all elements of the list but only the first ones.
  (all elements are affected because the #withAllChildrenBreadthFirstDo: )

[plantec]

Not a topple bogue.
The label needs to receive the selection event and let keep it simple.
For your use case a solution is to change the way the selection is drawn or to mask it completely as follow:

group := ToCheckableGroup new.
moveListElement := ToListElement new.
moveListElement selectionOption masked: true.
....

[plantec]

With the standard primary selection, the solution is to add the stamp #'ignore-selection' in the label (the label skin already takes this stamp into account) :

...
	moveButtons do: [ :e | 
		group register: e. 
		e label addStamp:#'ignore-selection' ].
...

[Nyan11]

Should the ignore selection stamp be default on button labels ?

[plantec]

It is not just for button labels, it is used for cases like the one you noticed for the raw theme.
Another theme can manage that case differently or can use different stamps.
So, there is no default :)

[Nyan11]

If i understand correctly, this is not a bug but a missusage of the raw theme.
And each theme must come with its own guideline and default values.

For example, the ToRawTheme must come with a documentation explaining that you need to insert a dedicated stamp if you want to add a button to a list.

Is it ok to expect that a theme must include its own documentation ?

If yes, what does it have to take to be readable and accessible ?

I have another question: How do we distinct the bugs created by Toplo and the bugs created by the theme ?

[plantec]

the theme problem/bug/misusage comes from the RawTheme kind of skin within dedicated classes. It is difficult to take the context correctly into account. (here the context is a label with its own skin in a button with its own skin in a selected node (which selection element has its own skin) in a list. This is a good example to explain the benefit of a style sheet.
So, a theme should cover the case you noticed without a dedicated stamp but, it is not obvious and it takes time to do it nicely (btw, what solution would you adopt ?).
As you imply, this is not a cool solution because it is hidden and has to be documented.

If yes, what does it have to take to be readable and accessible ?

all the used stamps :))
but wait, normally, it should work correctly out-of-the-box for existing widgets. A user will not update it every day. Do you change the css of your web applications ?

I have another question: How do we distinct the bugs created by Toplo and the bugs created by the theme ?

A bug could be that the skin events are not sent correctly or in a bad order. Here, one can fix the issue by changing the skin, so one can say that it is a skin bogue, or this skin issue. But in the general case, I don't know

[plantec]

With the standard primary selection, the solution is to add the stamp #'ignore-selection' in the label (the label skin already takes this stamp into account) :

...
	moveButtons do: [ :e | 
		group register: e. 
		e label addStamp:#'ignore-selection' ].
...

Finally, I find the use of stamp not elegant indeed.
Now, any skin event can be prevented/allowed with BlElement>>preventSkinEventClass: and allowSkinEventClass:
Since the selection skin events management is particular (they are send recursively breath first) the BlElement api includes also #preventSelectionSkinEvents and #allowSelectionSkinEvents.

moveListElement nodeBuilder: [ :node :move :holder |
	...
	moveButtons do: [ :e | group register: e ].
	node preventSelectionSkinEvents.
	node addChild: moveElement
].