Make isFocusable() virtual

[GoogleCodeExporter]

In most situations, the three checks for isFocusable() are great, but there
is one case that came up with a widget in my project that would be easy to
resolve, but is messy to work around currently (although it is possible
to). Because of this case, I propose that isFocusable() be made virtual in
future versions of the GUIChan API.

Here's a situation where overloading isFocusable() would be desirable:

Let's say that I have a text field which normally isn't visible until it's
focused to the user. For instance, let's say that we're designing a field
that pops up like Firefox's find bar at the bottom of the window, or we
have some chat client which lets the user know that they can type by making
the field visible, then hiding it when you can't type. Currently, if we
tried to tab to a widget that behaved like this, the focusHandler would
skip over it.

Currently, if we are to get the desired behavior and allow tabbing into
this widget, we must do one of a few things:

1. Write a helper widget that's always visible, but has no dimensions and
doesn't draw, to take focus, then hand it over to the desired widget, which
would then disable the helper widget (a Robin Hood and Friar Tuck kind of
situation). However, this causes a lot more bloat to your running code, as
all you really need is just one simple function call, although the
implementing code to do this workaround isn't too complex.

2. Override the drawing method, write a few state functions to keep track
of it, and never set it invisible. But this here can cause a problem with
mouse control, as we might not want to set focus when clicked on, so it's a
much less ideal solution. Also, this requires a lot more coding than
solution 1, although you do have a lot less overhead on run time.

3. Any other solution variation you can think of. But in any case, from the
programming perspective, being able to override isFocusable() itself would
save anyone who has any unexpected corner cases like these to be able to
work through them without the need for a lot of complex code work around.

I hope this is done for the next release, as this should be a simple issue
to address for future versions of GUIChan.

Original issue reported on code.google.com by irar...@gmail.com on 15 May 2009 at 5:41

[GoogleCodeExporter]

The only thing to be carefull about is calling virtual functions from 
constructors. 
setFocusable is a function that is typically called from constructers, hence it 
should not be virtual, but isFocusable might be a function that is never called 
from 
constructors. In that case, I don't see any harm in making the function virtual.

Original comment by olof.nae...@gmail.com on 15 May 2009 at 9:42

[GoogleCodeExporter]

Thanks. I didn't think it would be an issue, and I actually wasn't wanting the
getFocusable or setFocusable to be virtual, since I don't see any reason to 
make them
virtual, especially if you're able to set your own isFocusable conditions.

BTW, great work on the library. I currently run an open source project which 
forked
from another project which uses GUIChan over a year ago. We were considering
switching to Agar, and might still in the future, but the only reason I can 
think of
at the moment would be to work off of a thread safe library, since all of the 
horrors
we were told existed with the library, after we worked through them, didn't 
seem to
be your guys' fault at all. In fact, this is probably the first issue I ever 
ran into
with this library where I've been less than satisfied. Glad to see that you 
consider
this to be a decent idea.

Original comment by irar...@gmail.com on 16 May 2009 at 6:08

[GoogleCodeExporter]

I think the problem is that isFocusable is trying to do way too much. It should 
be a
symmetric function with setFocusable. The additional checks are probably used 
when
tabbing through the GUI, but they should really be moved there then.

About tabbing to a hidden widget, I don't think that should be a supported 
use-case.
Also, the workaround with an additional hidden widget that takes tab focus 
should be
fine. Supporting this through a virtual isFocusable() method sounds like a hack.

Original comment by b.lindeijer on 17 May 2009 at 9:36

[GoogleCodeExporter]

I would argue that what you're proposing is the bigger hack when the current
conditions are the most common use case. The matter of fact is that the use 
scenario
for which it represents is so overwhelmingly common that ignoring these 
coincidental
cases that happen to be true in the vast majority in order to satisfy an 
outlier for
use is not the answer. Not to mention, that if things were done the way you're
proposing, it'd also lead to a bit of code bloat around focus events, since 
you'd
need to add an extra variable to keep track of the old visibility state, then 
switch
back to it when those conditions aren't met, essentially just dodging around a 
bit
while essentially implementing what it already does rather elegantly.

So, let's put it another way and look into some other fields of study. If we're 
to
look at physics, Einsteinien physics is by far more accurate than Newtonian 
physics,
yet we still use Newtonian in most calculations. Why? Because the formulas are 
a lot
simpler, and in those cases, the loss of precision doesn't make a significant 
enough
difference for the situation that it's used in to warrant using it. In 
computing,
likewise, we have data types like floating points which also are hacks in a lot 
of
senses, since they don't represent all numbers accurately, but it's been good 
enough
that the performance gained by using an imprecise version for all cases is good
enough for nearly all cases.

Now, as for saying that working around the issue is good enough, I would argue 
that
it is not, and I'll go back to the Newton vs. Einstein again to try to make my 
point
on this. While Newtonian is good enough for most situations, it doesn't fill 
the gap
for everything, obviously, or else we Einsteinien physics never would have 
gotten any
ground at all. Likewise, we have different formulas to use within quantum 
physics as
well that may not work ideally on the macro scale, but give us more precision 
that we
need for those given scales. It'd be entirely inappropriate to interchange each
formula when the conditions which we would use that formula don't match up with 
the
conditions that we've got. So, in the same sense, being able to make the 
contents of
isFocusable() virtual so that different widgets can implement the behavior as 
they
need it is better than forcing the user to have to patch around it.

However, I will concede that isFocusable() as a function is a bit off when 
compared
to how other getter functions work within guichan. So, I don't think that it 
would be
too bad of an idea to move the current contents to a new function, for instance:
canFocus(), and make it virtual, then make isFocusable() behave like any other 
getter
function in GUIChan (although I personally don't see isFocusable being all that
useful if it was done that way). And I will also admit that if you make the 
contents
of isFocusable() virtual, whatever the future function would be called, you are
opening the API to developers abusing it, but so long as you want to be an open 
API
that people can extend, that's just a risk that you'll need to take, since 
anything
can be used in a way that it wasn't intended to, with some uses being better 
than
others. However, I personally see opening up the conditions to make a widget
focusable as having more positives than negatives for GUIChan, and I hope we can
agree on that point at least.

Original comment by irar...@gmail.com on 19 May 2009 at 4:35

[GoogleCodeExporter]

Considering that for 0.8.x, isFocusable() just ands three booleans together
(mIsFocusable, isVisible(), isEnabled()), why can't you just make them all 
true? If
you want the widget to be hidden, then override draw and make it a nop when you 
want
it invisible. Similarly, you can leave it enabled and ignore input events when 
you
want it disabled.

Original comment by jaxad0...@gmail.com on 19 May 2009 at 5:07

[GoogleCodeExporter]

Since this hasn't been responded to for a while, I'm just going to specify a 
bit 
more on why something should get done for this issue, while going through the 
options that I specified, and what the repurcussions of going through with that 
specific temporary fix does:

1. While this works for most situations, it's not a complete solution, as there 
are 
still restrictions which can't be accomplished this way, not to mention that 
it's a 
bit more bloated. For instance, if we're implementing a stack for focus so that 
focus can get returned to the item which had focus previous to the current 
focus, 
this solution fails to be able to handle this properly. As for using this 
workaround, in my opinion, this signifies that you're mainly just trying to 
work 
around the problem for now, and can easily remove the temporary fix when it's 
properly taken care of. So this fix would satisfy users which choose this route 
completely.

2. So, in my opinion, if you approach this directly like I said, that person is 
a 
bit of an idiot and doesn't know what they're doing. The smarter way would be 
to 
just fork the library and make the fix yourself. You'd save a lot of code that 
way, 
but it'd be at the expense of having to constantly be fixing up compatibility 
with 
the main library (not to mention that it is a bit unfriendly in some respects).

3. If the user is doing this, then this fix won't completely satisfy them. 
Unlike 2, 
there's no guarantee that whoever's doing this is an idiot, and may have actual 
reasons for doing what they're doing. Depending on the situation, it might be 
more 
profitable to actually fork the GUIChan project to do what you're needing if 
you're 
in this situation, but it depends a bit on what sort of modifications you're in 
need 
of. No matter what fix you do for this issue, it won't solve everything that 
this 
user will need.

If this issue is in hiatus right now because of a debate over whether the 
function 
should be virtual or if the core library should get changed,  then it'd be nice 
to 
know, but I hope that this summary of the three situations that I could see 
earlier 
helps point out why this issue would need to get resolved, and why only 
solution #1 
was ever discussed (since the previous commenter didn't seem to get why #2 was 
never 
mentioned. I included it for completeness, not because it was a smart thing to 
do.)

Original comment by irar...@gmail.com on 17 Jun 2009 at 5:59