2

I'm currently trying to add some custom controls to a panel in winforms. Every control will be docked and build something like a "list". Now i'm trying to implement a feature to select/deselect every control. Its working fine, my problem is that it seems to be very slow sometimes. Currently its about ~50 custom controls in the panel.

modtable.Click += (s, e) =>
{
    foreach (Control m in pnl_ucMods.Controls)
    {
        if(m is ModTableEntry)
        {
            if(m != modtable)
            {
                ((ModTableEntry)m).BackColor = SystemColors.Control;
            }
            else if (m == modtable && m.BackColor == SystemColors.Control)
                m.BackColor = SystemColors.ActiveCaption;
            else
                m.BackColor = SystemColors.Control;
        }
    }
};

Whenever i click on one of the controls it will change the backcolor, on a second click it will change it back but thats only working if i wait like a second before i click again. If i click to fast, nothing happens and i have to click again. I understand that winforms is not designed to have tons of controls and i understand that foreach will need some time to loop through all the controls, but maybe someone here has a small idea how to improve the code and maybe solve this problem.

tl;dr

Click on one of the custom controls in the panel will change its backcolor. (Selected)

Every other control will change the backcolor too (deselected)

If the clicked control is already selected, it will deselect.

EDIT: A small example to test that problem. Just create a new project, add the code and call it.

private void addPanels()
{
    Panel newPanel = new Panel();
    newPanel.AutoScroll = true;
    newPanel.Dock = DockStyle.Fill;
    this.Controls.Add(newPanel);

    for (int i = 0; i < 50; i++)
    {
        Panel childPanel = new Panel();
        childPanel.Size = new Size(100, 30);
        childPanel.Dock = DockStyle.Top;

        childPanel.Click += (s, e) =>
        {
            foreach (Control p in newPanel.Controls)
            {
                if (p is Panel)
                {
                    if (p != childPanel)
                        ((Panel)p).BackColor = SystemColors.Control;
                    else if (p == childPanel && p.BackColor == SystemColors.Control)
                        p.BackColor = SystemColors.ActiveCaption;
                    else
                        p.BackColor = SystemColors.Control;
                }
            }
        };
        newPanel.Controls.Add(childPanel);
    }
}
Backslash
  • 99
  • 1
  • 10
  • @JohnCarpenter basically yes. It should be possible to click on every control. A click on it will change all other controls in that panel and the control that gots clicked. Is that not the right way to do that? Is there a better way for it? – Backslash Jul 12 '16 at 13:04
  • Where are you adding the `Click` event handler? Why not make it a function rather than a lambda? I wonder if you aren't adding the event handler multiple times inadvertently. – D Stanley Jul 12 '16 at 13:16
  • I would add some `Debug.Writeline` calls before, during, and after the loop (with a timestamp) so you can see when it gets called, how much work it is doing, and when it is done. You may have a situation where it is happening more than you expect. – Bradley Uffner Jul 12 '16 at 13:30
  • @DStanley i'm adding the click event handler at the same point i'm adding the custom controls to the panel at runtime. – Backslash Jul 12 '16 at 13:31
  • Forget about custom control and post a simple code to reproduce the problem using some panels hosted in another panel. – Reza Aghaei Jul 12 '16 at 13:37
  • @BradleyUffner i tried that and it seems like every debug line hits the exact same time. Makes me wondering why i have this problem. `START: 12.07.2016 15:38:48 CLICK: 12.07.2016 15:38:48 END: 12.07.2016 15:38:48` I deleted all the "clicks" but start, every single click and the end are at the same second. – Backslash Jul 12 '16 at 13:40
  • @RezaAghaei i edited the start post and added a small piece of code to test the problem. – Backslash Jul 12 '16 at 13:52
  • Not that its the main issue but you can remove the `m == modtable` from the else if since it always will be equal by that point – Sayse Jul 12 '16 at 13:55
  • @Sayse thats true, i removed it but obviously nothing changed. :P Just a little bit more "clean".^^ – Backslash Jul 12 '16 at 13:57
  • 1
    It takes less than 1 millisecond. What's your expectation? – Reza Aghaei Jul 12 '16 at 14:05
  • I just tried with 500 panels, it works fine, what's in the paint() function of ModTableEntry? – Thomas Ayoub Jul 12 '16 at 14:06
  • @RezaAghaei for me its only working fine if you wait just a little bit before clicking on the same control again. If you are too fast it does not deselect the control. Maybe you are not clicking fast enough? I tested Alexander Petrov's answer and that solves my problem. I can click as fast as i want and its selecting/deselecting how it should. – Backslash Jul 12 '16 at 14:24
  • @ThomasAyoub read the comment above with the answer for Reza. – Backslash Jul 12 '16 at 14:24
  • 1
    @Backslash It would be DoubleClick if you click too fast. – Reza Aghaei Jul 12 '16 at 14:27
  • @RezaAghaei ah so if i click too fast its waiting to see if it is a doubleclick, that explains the problem and it also explains why its working fine with mousedown instead of click. Thank you, never thought about that doubleclick part..^^ – Backslash Jul 12 '16 at 14:30
  • 1
    Anonther problem in your code is you are using `childPanel` in event handler delegate. Instead use `var sender = (Control)s;`. It may cause some problems, because you create new child panel in each course of loop. – Reza Aghaei Jul 12 '16 at 14:48

1 Answers1

3

Use MouseDown event instead of Click event.

When you click twice too fast, it would be a DoubleClick event and no other Click event will raise.

With your permission, Reza.

Alexander Petrov
  • 13,457
  • 2
  • 20
  • 49