Fix for issue #839 (CheckComboBox ListChangeListener fires off twice after selection change)

#684 Open
Repository
negste
Branch
issue_839
Repository
controlsfx
Branch
default

Bitbucket cannot automatically merge this request.

The commits that make up this pull request have been removed.

Bitbucket cannot automatically merge this request due to conflicts.

Review the conflicts on the Overview tab. You can then either decline the request or merge it manually on your local system using the following commands:

hg update default
hg pull -r issue_839 https://bitbucket.org/negste/controlsfx
hg merge issue_839
hg commit -m 'Merged in negste/controlsfx/issue_839 (pull request #684)'
Author
  1. Stefano Negri
Reviewers
Description

Fixed by acting directly on the item's boolean property instead of on the control's checkmodel

Comments (1)

  1. Stefano Negri author

    I would like to add a comment to ask if I am missing some potential problems that might be revealed by the original issue (#839).

    I have found that the place where the event fires for the second time is in

    CheckBitSetModelBase.updateMap()
    

    and specifically here:

                // this is where we listen to changes to the boolean properties,
                // updating the selected indices list (and therefore indirectly
                // the selected items list) when the checkbox is toggled
                booleanProperty.addListener(new InvalidationListener() {
                    @Override public void invalidated(Observable o) {
                        if (booleanProperty.get()) {
                            checkedIndices.set(index);
                            final int changeIndex = checkedIndicesList.indexOf(index);
                            checkedIndicesList.callObservers(new NonIterableChange.SimpleAddChange<>(changeIndex, changeIndex+1, checkedIndicesList));                            
                        } else {
                            final int changeIndex = checkedIndicesList.indexOf(index);
                            checkedIndices.clear(index);
                            checkedIndicesList.callObservers(new NonIterableChange.SimpleRemovedChange<>(changeIndex, changeIndex, index, checkedIndicesList));
                        }
                    }
                });
    

    Acting on the CheckModel seemed the most natural thing to do, but it had the side effect of double-firing the event; so my solution was to act directly on the item's boolean property.

    I'm writing this comment because I am wondering if what I have proposed with this PR is just "bypassing" a potential problem in CheckBitSetModelBase. Thank you and regards