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.