clickable QLabels?

Darkware

I managed to piece together enough code to get a clickable label working. Now I want to make more buttons using that same class. In order to get each button doing something different, I have to be able to tell which button was clicked. This is where I'm having trouble.

The way I decided to try to accomplish this was by:
a) creating a variable called whichButton for the class
b) setting whichButton to "red" in mainWindow.cpp
c) passing whichButton to redClicked() from within the connect() command within ClickableLabel.cpp
d) having redClicked() see what whichButton is = to in an if() to determine which button was clicked, and from there do whatever.

In the code below, all code related to whichButton is commented out. This is to show that the code works fine for the one button I have without any of the whichButton code. If you uncomment the code however, it stops working. The button is not clickable any more, or at least has no reaction when clicked.

Due to my being new to Qt, I realize I may be going about this the wrong way. Either way though, I'd like to know what I'm doing wrong here or a better route for me to take if there is one. Thanks.

In mainWindow.cpp:

ClickableLabel *apicture = new ClickableLabel("<img src='myBG.png' />");
//apicture->whichButton = "red";

In ClickableLabel.h:

#include <QtWidgets>
#include <QString>

class ClickableLabel : public QLabel
{
Q_OBJECT
public:
	explicit ClickableLabel( const QString& text="", QWidget* parent=0 );
	~ClickableLabel();
	//QString whichButton;
public slots:
	void redClicked(/*QString whichButton*/);
signals:
	void clicked();
protected:
	void mouseReleaseEvent(QMouseEvent* event);
};

and finally in ClickableLabel.cpp:

#include "ClickableLabel.h"
#include <QMouseEvent>

ClickableLabel::ClickableLabel(const QString& text, QWidget* parent)
	: QLabel(parent)
{
	connect(this, SIGNAL(clicked()), this, SLOT(redClicked(/*whichButton*/)));
	setText(text);
}

ClickableLabel::~ClickableLabel()
{
}

void ClickableLabel::mouseReleaseEvent(QMouseEvent* event)
{
	(void)event;
	emit clicked();
}


void ClickableLabel::redClicked(/*QString whichButton*/)
{
	qDebug()<<"Clicked";
	//if(whichButton == "red button"){
		QMessageBox::about(this, tr("whatever"),tr("red was clicked"));
	/*}
	else {
		QMessageBox::about(this, tr("ddddd"),tr("else"));
	}*/
}

Chris Kawa

So a couple of topics here. Lets go one by one.

Due to my being new to Qt, I realize I may be going about this the wrong way

Looks like you are. If it's clickable like a button why not just use a QPushButton? You can make it flat to look more like a label. Also, you can put html links in QLabel's text and you will get a linkActivated signal when the link is clicked.

I'd like to know what I'm doing wrong here

You created a clicked() signal with no parameters. As long as it is connected to slot with no parameters that's fine. When you uncomment the parameter there's gonna be an error, because the signal does not pass any parameters and the slot expects one.

apicture->whichButton = "red";

There's no variable called whichButton in your class. It's a parameter to a slot.

Besides, it's a design flaw to put any instance dependent code into the class. What if someone places a 100 of those labels somewhere. Are you gonna make a 100 if/elses and modify the class every time someone adds a button? That's a bad idea for maintenance and scalability. The instance related code should be placed in "user's code" i.e. not inside that label class. There shouldn't be redClicked slot in a label class. It shoud be in the code using that label. Also calling it "redClicked" is not a good idea. What if you change the color? Slots should be usually named for what they do, not what they react to. For example:

SomeClass::SomeClass(QWidget* parent) : QWidget(parent)  //or wherever you create the labels
{
    ClickableLabel* redLabel  = new CLickableLabel("foo");
    ClickableLabel* blueLabel = new CLickableLabel("bar");

    connect(redLabel,  &ClickableLabel::clicked, this, &SomeClass::doSomething);
    connect(blueLabel, &ClickableLabel::clicked, this, &SomeClass::doSomethingElse);
}

void SomeClass::doSomething() {  qDebug() << "first label was clicked";  }
void SomeClass::doSomethingElse() {  qDebug() << "second label was clicked";  }

By the way - you can set images on the label using setPixmap. You don't need to use html for that.

Darkware

Yeah, I figured I might be going about it the wrong way. That's why I decided to ask. haha

I believe I looked up "how to add an image in Qt" or something and saw that one way to do it simply was via a QLabel and that's what sent me down that road instead of doing a QPushButton.

Believe it or not, I did try to write the connect() code outside of the class, but I couldn't get it to work. I eventually found code on a forum where it was written inside the class and it did work, so newbie me just accepted that that was the way it was supposed to be done even though it didn't seem right.

Either way, I took your advice, did a little research, and eventually came up with this code:

	QPushButton *redButton = new QPushButton();
	redButton->setStyleSheet("QPushButton{ border: 0px solid #000000;"
							 "background-image: url(myBG.png); }"
							 "QPushButton#redButton:pressed{ background-image: url(myBG.png); } ");
	redButton->setFlat(true);
	QPixmap pixmap("myBG.png");
	QIcon ButtonIcon(pixmap);
	redButton->setIcon(ButtonIcon);
	redButton->setIconSize(pixmap.rect().size());
	redButton->setFixedWidth(100);
	connect(redButton, SIGNAL(clicked()), this, SLOT(onClickRedButton()));

The style sheet was the trickiest bit. Without any style sheet at all, you can see the down state of the actual button underneath the image, which shouldn't be there. I just wanted the image to be the actual button. The syntax for style sheets is definitely not something I agree with. It can be written a few different ways, which is confusing, but nevertheless, I got it done and my button works.

Thanks for the help and everything you wrote out. Your patience is much appreciated. Someday I will be good enough to help others hopefully.