Winforms: Add support for tasks to ListViewGroup

Created on 7 Jan 2020  路  14Comments  路  Source: dotnet/winforms

  • You can set the LVGF_TASK flag on mask and set pszTask to a valid string and cchTask to the length of the string

Proposed API

This would add the following APIs to ListView and ListViewGroup. I modelled these after the ColumnClick event args/handler/method

public delegate void GroupLinkClickEventHandler(object sender, GroupLinkClickEventArgs e);
public class GroupLinkClickEventArgs : EventArgs
{
    public GroupLinkClickEventArgs(int column) { }

    public int GroupLink { get; }
}
public class ListView
{
    ...
    public event GroupLinkClickEventHandler GroupLinkClick { add; remove; }
    protected void OnGroupLinkClick(GroupEventArgs e) { }
    ...
}
public class ListViewGroup
{
    ...
    public string Task { get; set; }
    ...
}

Example

var listView = new ListView();
var group = new ListViewGroup
{
    Task = "Task"
};
listView.Groups.Add(group);
listView.GroupLinkClick += (sender, e) => MessageBox.Show("Link Clicked");

E.g.
Screen Recording 2020-01-04 at 07 01 pm

api-approved api-suggestion tenet-OS-compat

Most helpful comment

In my local implementation I went with the LVN_LINKCLICK. Although this would add more structs to the internal interop, this is no harm - they're not exposed and relatively simple.

I think making the distinction between the header and the task itself is important so I think LVN_LINKCLICK is the way to go

I agree

Example LVN_CLICKED in WndProc

case (int)User32.WindowMessage.WM_REFLECT_NOTIFY:
    User32.NMHDR* nmhdr = (User32.NMHDR*)m.LParam;
    switch (nmhdr->code)
    {
        case (int)LVN.LINKCLICK:
            NMLVLINK* link = (NMLVLINK*)m.LParam;
            string task = "Clicked";
            var group = new LVGROUPW
            {
                cbSize = (uint)sizeof(LVGROUPW),
                mask = LVGF.TASK,
                cchTask = (uint)task.Length
            };
            fixed (char* pTask = "Clicked")
            {
                group.pszTask = pTask;
                User32.SendMessageW(Handle, (User32.WindowMessage)LVM.SETGROUPINFO, (IntPtr)link->iSubItem, ref group);
                MessageBox.Show("Link Clicked");
            }
            break;
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

internal static partial class Interop
{
    internal static partial class ComCtl32
    {
        public struct NMLVLINK
        {
            public User32.NMHDR nmhdr;
            public LITEM link;
            public int iItem;
            public int iSubItem;
        }
    }
}
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

internal static partial class Interop
{
    internal static partial class ComCtl32
    {
        public unsafe struct LITEM
        {
            private const int MAX_LINKID_TEXT = 48;
            private const int L_MAX_URL_LENGTH = 2048 + 32 + 3;

            public LIF mask;
            public int iLink;
            public LIS state;
            public LIS stateMask;
            public fixed char szID[MAX_LINKID_TEXT];
            public fixed char szUrl[L_MAX_URL_LENGTH];
        }
    }
}
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;

internal static partial class Interop
{
    internal static partial class ComCtl32
    {
        [Flags]
        public enum LIF : uint
        {
            ITEMINDEX = 0x00000001,
            STATE = 0x00000002,
            ITEMID = 0x00000004,
            URL = 0x00000008,
        }
    }
}
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;

internal static partial class Interop
{
    internal static partial class ComCtl32
    {
        [Flags]
        public enum LIS : uint
        {
            FOCUSED = 0x00000001,
            ENABLED = 0x00000002,
            VISITED = 0x00000004,
            HOTTRACK = 0x00000008,
            DEFAULTCOLORS = 0x00000010,
        }
    }
}

All 14 comments

The team had some preliminary discussions about enhancing the control.
Could you spare few minutes and provide answers to the following questions so we can take it to the next level:

  • Will VS Designer need to support the feature? If yes, describe _how_ you expect it to fun褋tion.
  • What impact will it have on accessibility?
  • Will this feature need to be localized or be localizable?

Video

  • Let's stop creating new delegates for the basic event pattern and just use EventHandler<T>
  • ~Is GroupLink the best name for the concept? Or is it some kind of index? If so, the name should reflect that.~ Resolved

C# namespace System.Windows.Forms { public class GroupLinkClickEventArgs : EventArgs { public GroupLinkClickEventArgs(int groupIndex); public int GroupIndex { get; } } public partial class ListView { public event EventHandler<GroupLinkClickEventArgs> GroupLinkClick; protected void OnGroupLinkClick(GroupEventArgs e); } public partial class ListViewGroup { public string Task { get; set; } } }

As a note, we have discussed whether we should continue using the established pattern of generating new types for event handlers (e.g. GroupLinkClickEventHandler) or use the "newer" pattern of generic handlers (i.e. EventHandler<T>).
It was resolved that where possible we should use the generic version to reduce the type bloat.

@RussKie @merriemcgaw I think the bot is applying pull request rules against issues now? Also this issue had waiting-author-feedback tag never removed even though it seems to have been accepted as a proposal.

Still waiting a response from Hugh. The API review is one of several hoops a proposal has to go through.

The bot was actually misbehaving before, and got recently fixed.

but why auto-close a proposal if its up for grabs? Also bot text is incorrect in the WinForms repo (which made me think its bugged) - in the WPF repo it speaks of issues when editing them, not of pull requests.

it speaks of issues when editing them, not of pull requests.

My bad, I failed to update the wording. Fixed

@lonitra - keep an eye on this one, it could be another interesting one for ya 馃檪.

@hughbe could please remind what public int GroupLink { get; } is for?

@RussKie I modelled the API after ColumnClick, so it is an index to the group in ListView.Groups.

Perhaps a name like GroupIndex would be better

Thank you, I have updated @terrajobst's code snippet.

I've been looking at some documentation and I found some things that might be interesting that I would appreciate some input on: LVN_LINKCLICK and NMLVLINK structure.

I found that if the ListViewGroup task is clicked on then it is in fact a LVN_LINKCLICK notification code, but to my understanding this Task property is simply the name of the task and not the link itself. The NMLVLINK struture provides some extra information about the link itself that I don't think we necessarily care about since it seems like it's up to the user to do whatever they would like (trigger a link or not) once they get the callback from us.

With this info, I am wondering if we should trigger the GroupLink listeners under a regular HITTEST, where we will be checking to see if a group with a task was clicked on, or under this LVN_LINKCLICK notification code.

A downside I'm seeing with a regular HITTEST is that I cannot isolate the task from the group header, the HITTEST will simply register saying the group header was hit whereas LVN_LINKCLICK is able to make this distinction between the task and the rest of the header.
The downside I'm seeing with LVN_LINKCLICK is we would need to add extra classes because while we don't need the other information provided by NMLVLINK, we do need iSubItem, so that will require adding both NMLVLINK struct and LITEM struct. There might be some other things I'm not aware of.

I think making the distinction between the header and the task itself is important so I think LVN_LINKCLICK is the way to go, but @JeremyKuhne @hughbe @weltkante I'd love to hear your thoughts on this as well. 馃檪

In my local implementation I went with the LVN_LINKCLICK. Although this would add more structs to the internal interop, this is no harm - they're not exposed and relatively simple.

I think making the distinction between the header and the task itself is important so I think LVN_LINKCLICK is the way to go

I agree

Example LVN_CLICKED in WndProc

case (int)User32.WindowMessage.WM_REFLECT_NOTIFY:
    User32.NMHDR* nmhdr = (User32.NMHDR*)m.LParam;
    switch (nmhdr->code)
    {
        case (int)LVN.LINKCLICK:
            NMLVLINK* link = (NMLVLINK*)m.LParam;
            string task = "Clicked";
            var group = new LVGROUPW
            {
                cbSize = (uint)sizeof(LVGROUPW),
                mask = LVGF.TASK,
                cchTask = (uint)task.Length
            };
            fixed (char* pTask = "Clicked")
            {
                group.pszTask = pTask;
                User32.SendMessageW(Handle, (User32.WindowMessage)LVM.SETGROUPINFO, (IntPtr)link->iSubItem, ref group);
                MessageBox.Show("Link Clicked");
            }
            break;
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

internal static partial class Interop
{
    internal static partial class ComCtl32
    {
        public struct NMLVLINK
        {
            public User32.NMHDR nmhdr;
            public LITEM link;
            public int iItem;
            public int iSubItem;
        }
    }
}
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

internal static partial class Interop
{
    internal static partial class ComCtl32
    {
        public unsafe struct LITEM
        {
            private const int MAX_LINKID_TEXT = 48;
            private const int L_MAX_URL_LENGTH = 2048 + 32 + 3;

            public LIF mask;
            public int iLink;
            public LIS state;
            public LIS stateMask;
            public fixed char szID[MAX_LINKID_TEXT];
            public fixed char szUrl[L_MAX_URL_LENGTH];
        }
    }
}
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;

internal static partial class Interop
{
    internal static partial class ComCtl32
    {
        [Flags]
        public enum LIF : uint
        {
            ITEMINDEX = 0x00000001,
            STATE = 0x00000002,
            ITEMID = 0x00000004,
            URL = 0x00000008,
        }
    }
}
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;

internal static partial class Interop
{
    internal static partial class ComCtl32
    {
        [Flags]
        public enum LIS : uint
        {
            FOCUSED = 0x00000001,
            ENABLED = 0x00000002,
            VISITED = 0x00000004,
            HOTTRACK = 0x00000008,
            DEFAULTCOLORS = 0x00000010,
        }
    }
}

I think making the distinction between the header and the task itself is important so I think LVN_LINKCLICK is the way to go

I agree as well

Was this page helpful?
0 / 5 - 0 ratings