smokeAndMirrors
smokeAndMirrors

Reputation: 155

Change background colour of button after multiple buttons are clicked

I am using Visual C# 2010

I have a windows form with about 24 buttons plus a 'Finish' button

Currently the Finish button is red. I want the colour of this button to change to green once all other buttons have been clicked.

When the user clicks button 1 -24 that button changes to green, so I know how to change the background on a click event.

I have a boolean for when each button is clicked.

This is the code I have so far:

if (button1Clicked && button2Clicked ...)  // I have button3 through button24 as well
{
    finishButton.BackColor = Color.Green;
}

However the button remains Red when all 24 buttons have been clicked. I have sent the output of button1 to the debug writeline and I know that the boolean changes from false to true when the button is clicked. I have made the finish button public in the modifiers option.

Am I missing a step? I thought this should have been easy to accomplish.

EDIT


public partial class Form2 : Form
{
    public bool button1Clicked = false;
    .... // and other buttons are allocated the same way
}


private void button1_Click(object sender, EventArgs e)
{
    String textForClipboard = String.Format("{0}_S1_{1}", Form1.dateValue, button1.Text);
    Clipboard.SetText(textForClipboard);
    button1.BackColor = Color.Green;
    button1Clicked = true;
    System.Diagnostics.Debug.WriteLine(button1Clicked);
}

EDIT


Where exactly should I have the if loop? I currently have it in this method

namespace WindowsFormsApplication1
{
    public partial class Form2 : Form
    {
        ....

        public Form2(Form parentForm)
        {
            InitializeComponent();
            mainForm = parentform;
            ....

            if (button1Clicked)
            {
                finishedButton.BackColor = Color.Green;
            }
        }
    }
}

Upvotes: 2

Views: 2619

Answers (3)

trashr0x
trashr0x

Reputation: 6575

A small modification to globetrotter's answer. Change the allButtonsClicked() to return bool instead of void:

private bool allButtonsClicked()
{
    const int totalButtons = 23;
    int counter = 0;
    for (int i = 0; i < totalButtons; i++)
    {
        if (allButtons[i])
        {
            counter++;
        }
    }
    if (counter == totalButtons)
    {
        return true;
    }
    return false;
}

So you can then do:

private void button_Click(object sender, EventArgs e)
{
    var indexButton = int.Parse((((Button)sender).Name.Substring(6))) - 1;
    allButtons[indexButton] = true;
    if (allButtonsClicked())
    {
        finishButton.BackColor = Color.Green;
    }
}

This just results in the code being more readable and understandable in every-day language: "Whenever a button is clicked, check if each of the 24 buttons has been clicked at least once. If yes, then change the BackColor of finishButton to Green" and centralises your logic in one place rather than two.

Upvotes: 2

globetrotter
globetrotter

Reputation: 1047

This is a quick'n'dirty solution, which however works. It assumes that your 24 buttons are named (not their text) button1 to button24.

1) put your 24 buttons in a GroupBox (e.g. groupBox1)

2) put this in partial class Form2 : Form

bool[] allButtons = new bool[24];

private void button_Click(object sender, EventArgs e)
{
    var indexButton = int.Parse((((Button)sender).Name.Substring(6))) - 1;
    allButtons[indexButton] = true;
    allButtonsClicked();
}

private void allButtonsClicked()
{
    const int totalButtons = 23;
    int counter = 0;
    for (int i = 0; i < totalButtons; i++)
    {
        if (allButtons[i])
        {
            counter++;
        }
    }
    if (counter == totalButtons)
    {
        finishButton.BackColor = Color.Green;
    }
}

3) put this in Form2_Load()

foreach(Control ctl in groupBox1.Controls)
    {
        if (ctl is Button)
        {
            ctl.Click += button_Click;
        }
    }

What it does:

It loops through all the controls in groupBox1 when Form2 loads, checks if they are buttons and hooks all 24 buttons click events to button_click(). This way you don't need to declare 24 button clicks, since button_click() will trigger every time each of the 24 buttons is clicked.

We then have an allButtons bool array with 24 positions, all initially set to false. Each time button_click() triggers, we extract the "numerical" portion of the name of the button triggering button_click() at that given time (so for example if it's button1 triggering button_click() at than given time, we take substring "1" from "button1"), convert that to an int (so "1" becomes 1) and use that to set allButtons[0] = true (the array is zero-based, hence why 1 is used for index 0). When all 24 elements of allButtons become true (which is checked in allButtonsClicked()), we change finishButton's color to Color.Green.

Upvotes: 3

Krekkon
Krekkon

Reputation: 1347

First of all, be sure that the inner part of the condition is executed anytime. for example you can pop up a message box. Check only for 3 buttons for first, smaller test cases.

For other test, remove the whole condition and use only the color changing. Is this works? If not, then you have to play around that part for first.

If it is works, then go back to your condition, that will be the leaker. Check for first only one button clicking. And then two etc.

In that way you can easily find your answer anytime. Do it in small tasks, to find the issue.

Tick a debug point to the condition, and check which flag is false. Mouse over button1clicked etc, all should be true

Upvotes: 0

Related Questions