ToLabel doesn't update on model change

[tinchodias]

Hi! I had a conversation with @labordep about this behavior that I consider a bug in ToLabel but may be not... so I'm reporting it to know @plantec opinion.

Code:

model := ToLabelModel new.
model text:
    ((String loremIpsum: 50) asRopedText
         fontSize: 20;
         bold;
         foreground: Color blue;
         yourself).
aToLabel := model onWidget.
aToLabel openInOBlSpace.

Then evaluate any of these lines:

aToLabel text foreground: Color random.
aToLabel text clearAttributes: 1 to: 6 if: [ :_ | true ].
(aToLabel text from: 3 to: 10) foreground: Color random.

And the label only refreshes after sending aToLabel textChanged.

I saw that foreground: announces BlTextAttributeAdded, and looking for references I discovered that AlbTextEditor subscribes to it, to react. So that may be the fix.

In the previous snippet, I added:

aText := aToLabel text.
aText when: BlTextStringsInserted send: #textChanged to: aToLabel.
aText when: BlTextsDeleted send: #textChanged to: aToLabel.
aText when: BlTextAttributeAdded send: #textChanged to: aToLabel.
aText when: BlTextAttributesRemoved send: #textChanged to: aToLabel.

and it was fixed.

[tinchodias]

BTW, it may make sense to create a superclass in common for these BlText change announcements.

[tinchodias]

Talking with Enzo I realized the basic BlTextElement suffers the same issue... may this behavior be intentional?

textElement :=
    BlTextElement new
        text: 'A' asRopedText;
        yourself.
textElement openInNewSpace.

textElement text
    when: BlTextStringsInserted send: #textChanged to: textElement;
    when: BlTextsDeleted send: #textChanged to: textElement;
    when: BlTextAttributeAdded send: #textChanged to: textElement;
    when: BlTextAttributesRemoved send: #textChanged to: textElement.

[ 10 timesRepeat: [
    | aRandomSize |
    aRandomSize := (SharedRandom globalGenerator nextIntegerBetween: 1 and: 10) * 30.
    textElement text fontSize: aRandomSize.
    0.5 seconds wait ] ] fork

[tinchodias]

I mean, the BlTextElement doesn't refresh without the explicit subscription.

[plantec]

I'm not sure. I should check further why it is not already done like that.

Btw, the model stuff should not be used. I'm not sure these classes will remain in the futur (except for ToAlbum). Maybe spec adapters will play the role of these kind of view-model

[labordep]

I do aToLabel textChanged after each text attribute modifications. for me its make sens. It's like modification of a png image pixel matrix : the top system don't need to observe each of theses evolutions and this is better to notify manually when this is ok.

[tinchodias]

Hi,

Working in Spec-Toplo, created a subclass of ToLabel, and added this initialize:

initialize

    super initialize.

    self whenTextReplacedDo: [ :new :old |
        new
            when: BlTextStringsInserted send: #textChanged to: self;
            when: BlTextsDeleted send: #textChanged to: self;
            when: BlTextAttributeAdded send: #textChanged to: self;
            when: BlTextAttributesRemoved send: #textChanged to: self.
        old unsubscribe: self ]

to fix this problem

[tinchodias]

@labordep okay, it's the approach as a List widget, that isn't subscribed to the items collection changes... I mean, similarly, the user has the responsibility to tell the widget it was to refresh.

[plantec]

hello @tinchodias why do you need to create a ToLabel subclass ? I think we should avoid this kind of evolution for now

[tinchodias]

Yes, it's a valid objection. I could rollback if it was really bad idea. The requirements from Spec to adapt a ToLabel to be a SpLabelPresenter are:

1. the Spec label can be enabled or disabled, and the Toplo widget which changes foreground color of the text
2. the Spec label sets and retrieves Strings from the Toplo widget First, I had an adapter for (2) and a dresser for (3), but it looked better as subclass of ToLabel

[plantec]

it's good to implement spec adapters. we can see was is lacking. regarding enable/disable, it will be integrated for all widgets. The look change will be implemented through a theme/skin mechanism. For string access, you can add it in ToLabel. Thanks!

[plantec]

Anyway, continue with subclassing. finally it is good as we can immediately see what is wrong or lacking :)

[tinchodias]

@plantec do you want that I push the enable and string API to ToLabel? or I continue with this approach with Spec backend and we push them before