On 9/3/21 8:14 PM Greg Ercolano wrote:
On 9/3/21 8:58 AM, Bill Spitzak wrote:
I agree, and your fix is probably
correct.
I think that show() and hide() in value() was
because I was worried that the tabs could get into
some kind of incorrect state where the number that
are visible is not one, and this is trying to fix
that. I think now that is probably a mistake, such
states should not happen, and it would be better
to just return the first visible one or null if
none are visible. If this is a problem it would be
better to fix it when there is an attempt to set
the value, rather than when it is read.
It might be good to enforce this "somewhere".. perhaps
in a protected or public
recalc() method that simply blesses the state of the
widget to sanity, just so that
if the user somehow leaves all groups hidden, e.g. in
fluid, Fl_Tabs() will at least
come up with a sane default.
Perhaps on the first call to draw(), as you kinda want
this to happen after the
widget ctor and after all the child widgets have been
added, but before a draw().
Probably in
every call to draw(), as it is now.
Okay, let's assume we have three methods:
(1) Fl_Widget *value() const {...} to get the current value
(as before, but 'const', w/o side effects)
(2) int value(Fl_Widget *) to set the value (as before)
(3) Fl_Widget *recalc() to "calculate" the correct
visibility of children (returns the visible child)
Details:
(1) This method would be 'const' (ABI change) and return the
first visible widget or NULL. It would not make status
changes. Rather than returning NULL it could also return the
last child if no child is visible (this needs to be
investigated). It would return NULL anyway if the Fl_Tabs
group has no children.
(2) int value(Fl_Widget *) can be changed similar to what
Greg and I suggested, calling value() in the beginning would
be fine if this was changed as described in (1). This would
always make only one widget visible - unless the given
widget is not a child of the Fl_Tabs widget. This special
case is documented and would result in no visible child -
which would be "fixed" subsequently by calling recalc() in
draw().
(3) recalc() would do what value() did before (as a side
effect). It finds the first visible child and hide()s all
other groups. If no child is visible it show()s the last
child. recalc() will return the only visible child or NULL,
just like value() did before. The return value is
particularly useful if recalc() is used where value() was
used before, e.g. in draw().
The code in Fl_Tabs.cxx would need to be adjusted to call
the "right" method of these three where needed. For
instance, draw() would call recalc() instead of value() etc.
With these changes the FLTK core would work as before but
avoid some potential side effects. The documentation would
need some adjustments. User code would not be affected in
most cases. Only user programs that rely on value()
modifying the visibility of children and otherwise changing
(add, delete, hide(), show()) children might be affected.
However, since draw() calls recalc() this should be fixed in
most if not all cases.
The issue discussed here should be fixed as well, of course.
I'll try how this works out and post a patch (or PR) -
likely tomorrow. Comments and suggestions would be
appreciated, of course.
