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

ListBoxAssist.IsToggle - Ensure last clicked item is always selected #3349

Merged
merged 2 commits into from Oct 29, 2023

Conversation

nicolaihenriksen
Copy link
Contributor

Fixes #3235

As the last comment in the issue states, when explicitly applying ListBoxAssist.IsToggle=true, the expected behavior is most likely that the items should behave similar to a radio button group. This ensures that the user cannot do anything to end up in a situation where no elements are selected. However, there are still no guards around selection being "removed" from code.

To fix the latter part, we could attach a PropertyChangedCallback on the IsToggle DP, but we would probably also need to listen for changes in the ListBox.Items collection which I think is probably taking it a step too far. If the developer explicitly sets ListBoxAssist.IsToggle=true, let's assume that he/she also knows not to do anything weird.

@Keboo Keboo added this to the 5.0.0 milestone Oct 27, 2023
@Keboo
Copy link
Member

Keboo commented Oct 27, 2023

This works, and I get that in most cases we probably expect the radio button group style behavior, but I am wondering if a better fix. But I am thinking there are cases where "none" would be valid. I am thinking we might want to allow that case to still exist but have this be the default.

@nicolaihenriksen
Copy link
Contributor Author

nicolaihenriksen commented Oct 28, 2023

But I am thinking there are cases where "none" would be valid.

If you read the issue, you will see I had the same concern. So do we need an AP to allow the none case then?

EDIT: I have added the AP and updated the PR.

Default behavior is now changed so the selected item can not be deselected. Setting this new AP will allow it, basically falling back to the old default behavior.
@nicolaihenriksen nicolaihenriksen added demo app Items that relate to the demo application visual breaking change Items here have affected the visual look of controls. labels Oct 28, 2023
@Keboo
Copy link
Member

Keboo commented Oct 29, 2023

Yea something like that looks good to me.

@Keboo Keboo merged commit 4b14f71 into master Oct 29, 2023
2 checks passed
@Keboo Keboo deleted the fix3235 branch October 29, 2023 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
demo app Items that relate to the demo application visual breaking change Items here have affected the visual look of controls.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ListBox toggle can be unselected
2 participants