DeferredSetter setting wrong value.

[grokys]

@jkoritzinsky I'm trying to get MenuItem keyboard handling working correctly and I think I've run into a problem with the DeferredSetter code. To see the problem ,check out the fixes/1440-menu-navigation branch (commit 17ccb98) then:

1. Open the Menu page
2. Press Alt to select the menu
3. Press the down arrow twice to select the "Menu with _Submenu" menu

A breakpoint should be signalled and you should see from the call stack that Menu.IsSelected is being set to true but the DeferredSetter is setting it as false.

Wondered if you had any idea what could be wrong as I'm not so familiar with this code.

[jkoritzinsky]

@grokys I figured out what's going on:

1. Menu item is selected
2. Parent Menu Item (which is a SelectingItemsControl) is notified that the selected child is changed.
3. The SelectingItemsControl resets its SelectedItems list
4. All children of the SelectingItemsControl are marked as not selected, not just the children that are no longer selected (because of its behavior when a list implementing INotifyCollectionChanged raises the CollectionChanged event with the Reset type).
5. Update to menu item marking it as unselected is queued.
6. SelectingItemsControl re-selects the menu item.
7. Because the menu item is already marked as selected, it doesn't queue the update (without this behavior, the bugs in StackOverflowException in binding #855 come back)
8. The Menu item dequeues the de-selecting update and de-selects itself.

I tried a few different fixes, but the one below is the only one that didn't seem to fail any of the unit tests on my machine.

My fix (committed to the fixes/1440-menu-navigation branch):

• Make the default SelectedItems list be an AvaloniaList with its ResetBehavior set to Remove instead of Reset.
  • This lets the SelectingItemsControl only de-select the elements that should be de-selected.
• Fix a bug in SelectedItemsCollectionChanged that sometimes didn't deselect selected elements when they were removed from the SelectedItems collection.

This fix allows the IsSelected property to always maintain the valid value.

I tried a number of fixes on DeferredSetter, but every one brought back the issues in #855 since the pattern of property sets in that bug was "invalid, valid, invalid, valid, etc." whereas in this bug its "valid, invalid, valid, invalid, etc." and DeferredSetter was designed to linearize the sets and prevent reverting back to an "invalid" set, which all cases but this one was the intermediate value.

[grokys]

Awesome, thanks Jeremy! I will check that out this evening. Thanks for all the effort working that out, it's not easy is it?

[jkoritzinsky]

Yeah it's a bit of a pain haha. Everything related to DeferredSetter (the bugs it fixes, the internal state machine, assumptions it makes about data flow, etc.) is anything but straightforward. I need to write up some solid technical docs on how it works.