Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hackfix for calling SelectAll with SelectedItem binding #13868

Merged
merged 3 commits into from
Dec 8, 2023

Conversation

grokys
Copy link
Member

@grokys grokys commented Dec 7, 2023

What does the pull request do?

As described in #13676, calling SelectAll() on a ListBox with no current selection and a two-way SelectedIndex or SelectedItem binding in place, causes SelectAll() to fail. What was happening is:

  1. SelectAll() is called
  2. This selects all items, and updates the SelectedIndex/SelectedItem property on the SelectionModel
  3. This causes PropertyChanged to be raised for SelectedIndex/SelectedItem on the ListBox
  4. The binding system writes the new value to the source
  5. The binding system notices that the value has changed in the source
  6. The binding system writes the new value back to the target SelectedIndex/SelectedItem, even though this value hasn't changed
  7. Setting SelectedIndex/SelectedItem's semantics are such that setting them clears all other selected items

Step 6 is a bug in the binding system: we should not be writing an unchanged value back to the target due to a change in the source, and this will be fixed in the upcoming binding system refactor. However in the meantime, I think we need a hackfix as an interim solution. The easiest way to do this is to add the fix to SelectionModel.

Fixed issues

Fixes #13676

@grokys grokys added bug backport-candidate-11.0.x Consider this PR for backporting to 11.0 branch labels Dec 7, 2023
@maxkatz6 maxkatz6 enabled auto-merge December 7, 2023 10:07
@maxkatz6 maxkatz6 added this pull request to the merge queue Dec 7, 2023
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0042725-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Dec 7, 2023
@maxkatz6 maxkatz6 enabled auto-merge December 8, 2023 03:22
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0042771-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 added this pull request to the merge queue Dec 8, 2023
Merged via the queue into master with commit 0a6987b Dec 8, 2023
7 checks passed
@maxkatz6 maxkatz6 deleted the fixes/13676-selectall-with-selecteditem-binding branch December 8, 2023 07:20
grokys added a commit that referenced this pull request Dec 8, 2023
* Added failing tests for #13676.

* Add hackfix for #13676.

---------

Co-authored-by: Max Katz <[email protected]>
@maxkatz6 maxkatz6 added backported-11.0.x and removed backport-candidate-11.0.x Consider this PR for backporting to 11.0 branch labels Jan 17, 2024
maxkatz6 added a commit that referenced this pull request Jan 17, 2024
* Added failing tests for #13676.

* Add hackfix for #13676.

---------

Co-authored-by: Max Katz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ListBox.SelectAll sometimes fails to select items when SelectedItem is bound
3 participants