Implement Collapsible/Collapsed for ListViewGroup

[lonitra]

Fixes #3067

Proposed changes

• Add CollapsedState API to ListViewGroup

Customer Impact

• Customers will have the ability to make their ListViewGroups collapsible and collapse/expand items in the group

Screenshots

Before

After

Test methodology

• Manual testing
• Unit testing

TODOs

• Accessibility Improvements

Notes

• The user can either collapse item(s) through single click on chevron or double click on group header

Microsoft Reviewers: Open in CodeFlow

[Tanya-Solyanik]

Consider the following code:

lvg.Collapsible = true;
lvg.Collapsed = true;
lvg.Collapsible = false;

We will get into an invalid state. Should we force collapsed to false when setting Collapsible to false?

[lonitra]

Thanks for the catch! I think we should force Collapsed to be false whenever Collapsible is false. This way whenever users set Collapsible to true, Collapsed will initially be false.

[Tanya-Solyanik]

The alternative would be to ignore lvg.Collapsible = false; if Collapsed is already true. Do we have similarly connected properties in other controls? What are they doing?

[lonitra]

I looked into files like GridEntry.cs which has Expandable/Expanded and it seems that Expandable is read-only and cannot be changed externally. Internally, there doesn't seem to be a place where Expandable is altered from its initial value.

[RussKie]

Mostly nits

[RussKie]

@msftbot merge if @Tanya-Solyanik approves

[ghost]

Hello @RussKie!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

• I'll only merge this pull request if it's approved by @Tanya-Solyanik

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

[Tanya-Solyanik]

Waiting for conclusion of ISupportInitialized discussion, otherwise looks good.

[lonitra]

Something I wasn't acutely aware of until I pulled down the branch. If a listview contains items that aren't assign to any group then a "Default" group is added to the list. However this group isn't retrievable via listView1.Groups collection.

I noticed this also. When I was working on collapse through keyboard-only and sending group messages, windows gave default group an index of 0. It makes me think that we should also keep track of a default group as index 0 in the groups collection, but I'm not sure what the reasoning was for not including default group in the collection to begin with.

[weltkante]

but I'm not sure what the reasoning was for not including default group in the collection to begin with.

Because the user didn't add it. Its a user-maintained collection and you want to keep up the illusion of items existing without group. A group repeatedly appearing/disappearing in slot zero would be very disturbing to user code.

some more reasons:

• you don't want to force the default group onto someone who doesn't need it because he never adds items without group
• you don't want to re-index existing groups if you add items without group much later, e.g. during a user interaction (existing code may rely on indices staying constant)

I don't think you can change that behavior because it will be a breaking change for anyone looping over the groups for processing items.

[RussKie]

Aye, it places us in a weird place - a user can alter own groups but has no control over the default group. This would lead to strange UX.

In https://github.com/gitextensions/gitextensions we've bolted on a group collapsibility to ListView, and boy was it ugly! Looking again through the code I realised ListView has DefaultGroup property, but it is marked as internal:
https://github.com/gitextensions/gitextensions/blob/4504a6933ab46d7937516630f74fa9df442a827a/GitUI/UserControls/ExListView.cs#L45-L46

winforms/src/System.Windows.Forms/src/System/Windows/Forms/ListView.cs

Lines 760 to 770 in 7a54b53

	internal ListViewGroup DefaultGroup
	{
	get
	{
	if (defaultGroup == null)
	{
	defaultGroup = new ListViewGroup(string.Format(SR.ListViewGroupDefaultGroup, "1"));
	}
	return defaultGroup;
	}
	}

Further it the DefaultGroup handling is quite inconsistent and required us to jump through hoops:
https://github.com/gitextensions/gitextensions/blob/4504a6933ab46d7937516630f74fa9df442a827a/GitUI/UserControls/ExListView.cs#L299-L304

            // .NET ListView.Groups.Insert(...) implementation has a bug
            // It does not count the technical "Default" group from ListView when passing inserted
            // group index to native Win32 ListView.

            // ListView.Groups.Add(...) implementation on the other hand does count the
            // technical "Default" group correctly therefore it becomes broken after calling this method

[weltkante]

Aye, it places us in a weird place - a user can alter own groups but has no control over the default group. This would lead to strange UX.

You can expose it as a property, no need to make it part of the enumerable collection. The property can be either on the control or on the collection.

[RussKie]

Well done! Thank you

[M-Lipin]

Accessibility for ListViewGroup expanded/collapsed state and action is covered in #3224.

[Vino-Wang]

@lonitra Just out of curiosity, I was wondering if there is some keyboard combination to Expand/Collapse the ListViewGroup. For now, only can using mouse to click the Icon of Expanded/Collapsed or double-clicking on the whole Group.

[lonitra]

@Vino-Wang As of right now there's not. Ideally the groups would collapse/expand with left and right arrow key. I have made a work item for it here #3269

@M-Lipin Does #3224 cover collapse/expand through keyboard only? If not I can go ahead and take care of this 😊

[RussKie]

Looping in @Ryuugamine instead of Mikhail.