ImageButton should not be derived from Button

[GoogleCodeExporter]

Currently class ImageButton is derived from class Button. 

Better would be to have a base class of Button, e.g. ButtonBase, which 
contains most functionality of Button, to derive classes Button and 
ImageButton from. 

As it is, the caption of Button is not used in ImageButton. Also, the 
implementation of adjustSize uses the caption instead of the image. 

So class ButtonBase should have all members and functions of Button except 
the caption, setCaption, draw and adjustSize. Only those functions have to 
be implemented in Button. ImageButton would implement similar functions, 
derived from ButtonBase. 

Original issue reported on code.google.com by tobias.s...@gmx.net on 22 Oct 2008 at 3:42

[GoogleCodeExporter]

To get rid of this confusement, Qt has a QAbstractButton from which stuff like
QPushButton, QRadioButton and QCheckBox derive. But most stuff in in 
QAbstractButton
(icon, text, checkable, checked, etc.), the other classes barely add anything 
and are
more determining default values and style.

In the case of Guichan, in my opinion there isn't a good reason for 
gcn::ImageButton,
but gcn::Button should just support an icon and a text, and either could be set 
to
nothing.

Original comment by b.lindeijer on 22 Oct 2008 at 7:43

[GoogleCodeExporter]

I think an AbstractButton class would be a good way to handle this. Either way, 
could 
people decide on what direction they would like this to head in (maybe agree on 
an 
api) and then we could start coding it.

Original comment by aaron.r....@gmail.com on 10 Nov 2008 at 7:19

[GoogleCodeExporter]

If a Button would support both a caption and an image then we have the problem 
of
placing the caption and image, which can get very messy.

Original comment by olof.nae...@gmail.com on 10 Nov 2008 at 8:54

[GoogleCodeExporter]

If you have a caption AND an image, only one of these should be set for any 
instance. The advantage would be that it's easy to use, but the disadvantage is 
that 
you can't easily add other custom derived button classes. So, for that reason I 
would suggest implementing an abstract button class where button and 
imagebutton 
derive from. If someone needs a button that has both caption and image, he can 
add 
that as another derived class... 

Original comment by tobias.s...@gmx.net on 11 Nov 2008 at 7:15

[GoogleCodeExporter]

I don't see anything messy about placing the icon and text next to each other by
default. Somebody can always subclass the button to do something else with it.

Having three classes for this simple thing and still requiring the user to add a
fourth class just for combining icon and text sounds rather silly to me.

Original comment by b.lindeijer on 11 Nov 2008 at 8:18

[GoogleCodeExporter]

The main problem here is that you force something on the user, who just wants 
to 
make a custom button. Most users want a caption OR an image, not both. There 
are 
some who might need both elements. I need something else even, so I need to 
subclass 
in any case. But I don't like to be forced to have a caption that I don't want 
to 
use, an image I don't want to use and a draw method that I need to overwrite to 
implement my own behaviour: I want an abstract base class I can use for the 
behaviour, and then add my draw method as I need it. 

In most places, guichan aims for flexibility and reuse. Why do you want to go 
for 
something different here? If any user only wants his button, it's just not an 
issue 
if there is one or three classes. If someone needs to subclass, there IS an 
issue. 

Original comment by tobias.s...@gmx.net on 11 Nov 2008 at 8:39

[GoogleCodeExporter]

Note that in Guichan you _always_ have to subclass, since by default it looks
butt-ugly. So I think it is an advantage to the user that some classes share
functionality (and I'd say in the case of buttons this is very common in most 
toolkits).

If I want my buttons to look in a certain style, you're basically askming me to 
write
three custom subclasses. One for the TextButton, one for the ImageButton and 
one for
the ImageTextButton. And all of them would implement the exact same code which 
draws
the button border the way I want it to.

If there is one gcn::Button that provides the means to combine text and icon, 
then I
need to only customize it's drawing in one subclass and I'm done. What is the 
problem
with just ignoring the text or the icon in your button class?

Combining the two in one button would even allow me to easily provide the user 
with
the option whether he wants to show text or icons, or both, on his buttons. All 
I
need to do is hide some stuff rather than switch which instance of the class is 
used.

Original comment by b.lindeijer on 11 Nov 2008 at 8:56

[GoogleCodeExporter]

b.lindeijer,

What if we implement a Style class which allows you to dynamically control how 
widgets look
without having to subclass 
(http://guichan.sourceforge.net/forum/read.php?2,2603) ? Does that
adequately address your concern?

ps. I'm really glad we're talking about this now, once we agree on a method it 
should be pretty
straight forward for anyone to code up (just a matter of putting the time in).

Original comment by aaron.r....@gmail.com on 11 Nov 2008 at 1:33

[GoogleCodeExporter]

For sure the styling option would address part of my concerns, and I'm all for 
that.

Original comment by b.lindeijer on 11 Nov 2008 at 6:29

[GoogleCodeExporter]

Placing the caption and image could get messy if you for instance can place the
caption to the right of the image, to the left of the image, above the image, 
below
the image, aligned to the left side, aligned to the right side, aligned to the 
top,
aligned to the bottom...

Of course, we could skip a number of options and just go for three, a caption, 
an
image, a caption and then an image.

Personally I don't have a problem with an empty std::string inside a Button 
class if
the button were to support both a caption and an image. However, an abstract 
button
class could also be useful for the check box widget.

Original comment by olof.nae...@gmail.com on 11 Nov 2008 at 8:35

[GoogleCodeExporter]

QCheckBox is a subclass of QAbstractButton, and inherits the support for icons:
http://img521.imageshack.us/img521/274/checkboxeswithiconshf7.png

Anyway, as usual my opinion could be pretty tainted.

Original comment by b.lindeijer on 12 Nov 2008 at 12:33

[GoogleCodeExporter]

qt is a pretty solid library and given that a checkbox could be based on the 
AbstractButton class
(instead of including a string variable which might never get used in some 
child classes) i think
it's a pretty strong argument in that direction.

just my 2 cents.

Original comment by aaron.r....@gmail.com on 12 Nov 2008 at 1:47

[GoogleCodeExporter]

Note that Java, too, has an AbstractButton as the base for push buttons, radio
buttons, check boxes, etc. And the icon and text properties are also part of
AbstractButton and not of some TextButton, ImageButton, IconButton, 
TextImageButton
or whatever. I really think that's just overengineering something really basic.

Original comment by b.lindeijer on 12 Nov 2008 at 2:43