pharo-spec / Spec

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

Multi-selection in TreeTable doesn't work #1435

Open Nyan11 opened 1 year ago

Nyan11 commented 1 year ago

Hello,

Description

I encounter a bug while using SpTreeTablePresenter where the selection was not the same on the model and visually.

Examples:

SpTreeTablePresenter new
          beMultipleSelection;
          expandRoots;
          addColumn: (SpCompositeTableColumn new
                   title: 'Classes';
                   addColumn: (SpImageTableColumn new
                            evaluated: [ :aClass |
                                self iconNamed: aClass systemIconName ];
                            width: 20);
                   addColumn: (SpStringTableColumn evaluated: #name);
                   yourself);
          roots: { Object };
          children: [ :aClass | aClass subclasses ];
          whenSelectionChangedDo: [ :new | Transcript crShow: new selectedItems printString ];
          open
  1. I click on Object -> The transcript show: {Object}
  2. I press shift
  3. I click on Stream -> The transcript show: {Stream. Object. Exception. Path}
  4. I release shift
  5. I click on Exception -> The trascript should show {Exception}, but it doesn't.

gif bug tree table

Possible fix

I think the bug come from the method: #basicSelectionChanged: in the class: SpMorphicTreeTableAdapter

basicSelectionChanged: ann

| selection |

    selection := self presenter selection.
    ann newSelectedIndexes ifEmpty: [ ^ selection unselectAll ].
    (ann newSelectedIndexes difference: ann oldSelectedIndexes) ifEmpty: [ ^ self ].

    selection selectPaths: (ann newSelectedIndexes
        collect: [ :index | self widget dataSource pathFromIndex: index ]).

    self presenter isActiveOnSingleClick ifTrue: [
        self presenter doActivateAtPath: selection selectedPath ]

The method #difference: (line 7) return an empty collection in the example above. The line 7 should check if the new selection is different. In the example they are, but the command differnce: return an empty collection.

I suggest this solution, where we check the difference of new and old selection and the difference of old and new selection after. \ = difference:, | = union:.

basicSelectionChanged: ann

    | selection |
    selection := self presenter selection.
    ann newSelectedIndexes ifEmpty: [ ^ selection unselectAll ].
    (ann newSelectedIndexes \ ann oldSelectedIndexes) | (ann oldSelectedIndexes \ ann newSelectedIndexes) ifEmpty: [
        ^ self ].

    selection selectPaths: (ann newSelectedIndexes collect: [ :index |
             self widget dataSource pathFromIndex: index ]).

    self presenter isActiveOnSingleClick ifTrue: [
        self presenter doActivateAtPath: selection selectedPath ]

The method difference is also present in other SPEC class. Maybe this bug is present in them too:

Other

Pharo 11.0.0 Build information: Pharo-11.0.0+build.700.sha.53659ed8c20ddb4b644a58135325ff84be4e063e (64 Bit)

I used the Spec package directly present in Pharo image.