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

#684 Declined
Repository
negste
Branch
issue_839
Repository
controlsfx
Branch
default
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