Fl_Tabs

[rs]

Hi,

I wanted to check if anyone can replicate a bug I'm getting using an Fl_Tabs widget.

If I build some tabs object

Fl_Tabs *my_tabs = new Fl_Tabs(...);

with, say, n distinct tabs, and add some callback

my_tabs->add_callback(my_callback,data);

then the callback is only being called when switching to the first (n-1) tabs. I.e. if i have 4 tabs, "A","B","C", and "D", the callback is only called when switching to "A", "B" or "C".

I get this behaviour using v1.3.7 and the latest master version 1.4 on github. I am on a debian linux system.

Thanks,

R.

[Ian MacArthur]

Can you post a small, compileable, example that shows the effect, so we can see what you are doing?

This pretty much ought to Just Work.

 

[rs]

Ok, a minimal example couldn't replicate this issue.

But delving deeper I found the lines of code that caused this to occur.

My Fl_Tabs widget is a child of a window which I have derived from Fl_Double_Window in order to handle events through a derived handle(int) function.

In the derived handle function that I define I check which tab is currently active in an if{} statement. Checking this is causing the error.

To be explicit: here is the minimal version of the handle function which causes the issue

Note: my_tabs is an Fl_Tabs* type, and tabA is an Fl_Group* type.

int my_derived_window::handle(int msg){

    int ret = Fl_Double_Window::handle(msg);

    if (my_tabs->value() == tabA){

    // empty if  -- error occurs even with nothing inside here

    }

    return ret;

}

In contrast if I replace it with:

int my_derived_window::handle(int msg){

    int ret = Fl_Double_Window::handle(msg);

    return ret;

}

there is no issue. I have no idea how an "==" operator could be changing anything here...

Thanks,

R.

[rs]

Right, here is a minimal example that exhibits the issue:

#include <FL/Fl.H>
#include <FL/Fl_Double_Window.H>
#include <FL/Fl_Tabs.H>
#include <iostream>

Fl_Tabs *tabs;
Fl_Group *g1,*g2,*g3;

void test_cb(Fl_Widget*,void*){
    std::cerr<<"In callback"<<std::endl;
}

class mywin: public Fl_Double_Window{
        int handle(int);
    public:
        mywin(int X, int Y, int W, int H, const char* l):Fl_Double_Window(X,Y,W,H,l){};
};

int mywin::handle(int msg){

    int ret = Fl_Double_Window::handle(msg);

    if (tabs->value()==g1){}
    return ret;
}

int main() {

    mywin *win = new mywin(0.5*Fl::w()-200,0.5*Fl::h()-200,400,400,"Tab Test");
    tabs = new Fl_Tabs(10,10,win->w()-20,win->h()-30);

    g1 = new Fl_Group(10,35,tabs->w(),tabs->h()-35,"Tab A");
    g1->end();
    g2 = new Fl_Group(10,35,tabs->w(),tabs->h()-35,"Tab B");
    g2->end();
    g3 = new Fl_Group(10,35,tabs->w(),tabs->h()-35,"Tab C");
    g3->end();

    tabs->add(g1);
    tabs->add(g2);
    tabs->add(g3);
    tabs->callback(test_cb,NULL);
    tabs->end();
    win->add(tabs);
    win->end();
    win->show();
    return Fl::run();

}

Compiler issue maybe?

"g++ --version"   gives  "g++ (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0"

Thanks,

R.

[lifeatt...@gmail.com]

>then the callback is only being called when switching to the first (n-1) tabs. I.e. if i have 4 tabs, 

>"A","B","C", and "D", the callback is only called when switching to "A", "B" or "C".

I don't see the issue. I modified your example to show me which tab is active in the callback, and the callback is invoked for all tabs. (gcc 9.3.0 on Linux Mint 20). I'm uncertain what your test of tabs->value() was to accomplish in the mywin handler, you will receive messages for the "previous" tab (e.g. UNFOCUS) when switching to a new tab.

#include <FL/Fl.H>
#include <FL/Fl_Double_Window.H>
#include <FL/Fl_Tabs.H>

Fl_Tabs *tabs;
Fl_Group *g1,*g2,*g3, *g4;

void test_cb(Fl_Widget*,void*)
{
printf("In callback:");
if (tabs->value() == g1)
printf("tab1\n");
else if (tabs->value() == g2)
printf("tab2\n");
else if (tabs->value() == g3)
printf("tab3\n");
else if (tabs->value() == g4)
printf("tab4\n");

}

class mywin: public Fl_Double_Window{
int handle(int);
public:
mywin(int X, int Y, int W, int H, const char* l):Fl_Double_Window(X,Y,W,H,l){};
};

int mywin::handle(int msg){

int ret = Fl_Double_Window::handle(msg);

return ret;
}

int main() {

mywin *win = new mywin(0.5*Fl::w()-200,0.5*Fl::h()-200,400,400,"Tab Test");
tabs = new Fl_Tabs(10,10,win->w()-20,win->h()-30);

g1 = new Fl_Group(10,35,tabs->w(),tabs->h()-35,"Tab A");
g1->end();
g2 = new Fl_Group(10,35,tabs->w(),tabs->h()-35,"Tab B");
g2->end();
g3 = new Fl_Group(10,35,tabs->w(),tabs->h()-35,"Tab C");
g3->end();

g4 = new Fl_Group(10,35,tabs->w(),tabs->h()-35,"Tab D");
g4->end();

tabs->add(g1);
tabs->add(g2);
tabs->add(g3);

tabs->add(g4);

[rs]

>I don't see the issue. I modified your example to show me which tab is active in the callback, and the callback is invoked for all tabs. (gcc 9.3.0 on Linux Mint 20). I'm uncertain what your test of tabs->value() was to accomplish in the mywin handler, you will receive messages for the "previous" tab (e.g. UNFOCUS) when switching to a new tab.

Yeah I see that, thanks. Fortunately I've changed my program logic to get around this issue anyway now.

I am still getting the issue - confirmed with your modification. And have tried cleaning and recompiling my FLTK build (both 1.3.7 and current 1.4.0 snapshot). If you can't replicate it I can only chalk it up to some idiosyncrasy of my c/c++ implementation.

Thanks,

R.

[Greg Ercolano]

In the docs for Fl_Tabs, check the use of when() to control when the
callback is invoked, e.g.

https://www.fltk.org/doc-1.4/classFl__Tabs.html

See in particular the section entitled "Callback's Use Of when()"
where there's quite a bit written about this:

---

Callback's Use Of when()

As of FLTK 1.3.3, Fl_Tabs() supports the following flags for when():

    FL_WHEN_NEVER - callback never invoked (all flags off)
    FL_WHEN_CHANGED - if flag set, invokes callback when a tab has been changed (on click or keyboard navigation)
    FL_WHEN_NOT_CHANGED - if flag set, invokes callback when the tabs remain unchanged (on click or keyboard navigation)
    FL_WHEN_RELEASE - if flag set, invokes callback on RELEASE of mouse button or keyboard navigation

Notes:

1. The above flags can be logically OR-ed (|) or added (+) to combine behaviors.
2. The default value for when() is FL_WHEN_RELEASE (inherited from Fl_Widget).
3. If FL_WHEN_RELEASE is the only flag specified, the behavior will be as if (FL_WHEN_RELEASE|FL_WHEN_CHANGED) was specified.
4. The value of changed() will be valid during the callback.
5. If both FL_WHEN_CHANGED and FL_WHEN_NOT_CHANGED are specified, the callback is invoked whether the tab has been changed or not. The changed() method can be used to determine the cause.
6. FL_WHEN_NOT_CHANGED can happen if someone clicks on an already selected tab, or if a keyboard navigation attempt results in no change to the tabs, such as using the arrow keys while at the left or right end of the tabs.

---

[Greg Ercolano]

On 9/2/21 3:53 AM, rs wrote:

Right, here is a minimal example that exhibits the issue:

    Ah, OK, so here I take it the issue is even though the "A" tab is selected,
    clicking "C" does nothing.

    That seems like a bug and I can replicate here on Linux even with 1.4.x.

    If you use:

        tabs->when(FL_WHEN_CHANGED | FL_WHEN_NOT_CHANGED);

    ..it seems to work OK in that the callback is triggered /always/,
    but yeah, seems like the default behavior (do the callback on change)
    has an n-1 problem.

    Probably open an issue on github for this.

[Albrecht Schlosser]

On 9/2/21 4:36 PM lifeatt...@gmail.com wrote:

>then the callback is only being called when switching to the first (n-1) tabs. I.e. if i have 4 tabs, 

>"A","B","C", and "D", the callback is only called when switching to "A", "B" or "C".

I don't see the issue.

I do. It's really strange.

I modified your example to show me which tab is active in the callback, and the callback is invoked for all tabs. (gcc 9.3.0 on Linux Mint 20).

Linux Mint 20 here as well.
$ g++ --version
g++ (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0

I'm uncertain what your test of tabs->value() was to accomplish in the mywin handler, you will receive messages for the "previous" tab (e.g. UNFOCUS) when switching to a new tab.

Kevin, the issue is not that you will receive the value of the previous tab (which is true) but that the simple 'if ...' changes the behavior. If I'm not missing anything obvious that's a mystery (or really a C++ bug). Side note: I found another strange issue (a nonsensical compiler warning) with this g++ version. It's maybe really a bug.

I modified mywin::handle() like this:

int mywin::handle(int msg) {

int ret = Fl_Double_Window::handle(msg);

#if 1

if (tabs->value() == g1 && msg == FL_PUSH) printf("g1\n");

if (tabs->value() == g2 && msg == FL_PUSH) printf("g2\n");

if (tabs->value() == g3 && msg == FL_PUSH) printf("g3\n");

#endif

return ret;

}

If you change '#if 1' to '#if 0' the "callback behavior" changes as described by the OP in the way that "In callback" is printed only when you switch tabs to tab 1 or tab 2 but not to the last tab. That's the effect I'm seeing, and that is really unexpected. Particularly because the callback should already have been called by 'Fl_Double_Window::handle(msg);' when nthe following 3 statements are executed.

OK, I also built FLTK with clang and used debug and no-optimize (-g -O0) switches but the result is the same.

The above seems to rule out a compiler bug.

(Some time later ...)

New tests showed that calling tabs->value() appears to have a side effect that makes this issue happen. For anybody to reproduce the issue my further modified handle() looks like this:

int mywin::handle(int event) {

int ret = Fl_Double_Window::handle(event);

#if 0

void *pp = tabs->value();

// std::cerr << "tabs->value() =" << pp << std::endl;

#else

const char *pp = tabs->label() ? tabs->label() : "none";

// std::cerr << "tabs->label() = " << pp << std::endl;

#endif

return ret;

}

Note: if you comment out the output statements as shown above, make sure that the compiler doesn't optimize the calls away. It shouldn't do that with tabs->value() because it's not 'const', but anyway. Take care.

Now change '#if 0' to '#if 1' and watch the result:

(1) With '#if 0' tabs->label() is called and the code works as expected.
(2) With '#if 1' tabs->value() is called and the code fails to show "In callback" for the last tab.

BTW: I would have expected that Fl_Tabs::value() is a 'const' method but it isn't. Except returning the "current tab" value it does also show/hide tabs. This seems to have to do with this unexpected effect.

Still testing ...

[Greg Ercolano]

On 9/2/21 10:49 AM, Albrecht Schlosser wrote:

I do. It's really strange.

    I've been looking at this too, and it seems to me there's some strange
    behavior in the Fl_Tabs::value(Fl_Widget*) method, where somehow during
    the loop, the last widget is somehow becoming visible() before it's set visible.

    Haven't come to conclusions yet, but it sure seems in Fl_Tabs::value(Fl_Widget*)
    during the loop, calling hide() on the first child (Tab A) causes the /last/ child (Tab C)
    to suddenly become visible() for some reason, causing confusion in the loop.

    I'm suspecting the Fl_Group::handle() method code for FL_SHOW/FL_HIDE events
    to be the "culprit".

    I have to run away mid-debug, but that's where I left off.
    Anyone wanting to dig deeper, I suggest use the above to get past that
    first level of obfuscation, as that's a weird trip down the usual event rabbit hole.

[lifeatt...@gmail.com]

Drat. So it seems I misunderstood the bug. My apologies for the noise.

[lifeatt...@gmail.com]

'Taint the compiler. Building FLTK and the test app with Clang 10.0.0 also shows the issue.

[Bill Spitzak]

It does seem like a bug, calling value() should not have any side-effects. Especially because the tabs widget works even if nobody calls value().

On Thu, Sep 2, 2021 at 5:39 PM lifeatt...@gmail.com <lifeatt...@gmail.com> wrote:

'Taint the compiler. Building FLTK and the test app with Clang 10.0.0 also shows the issue.

--
You received this message because you are subscribed to the Google Groups "fltk.general" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkgeneral...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/fltkgeneral/51ad069b-dbbc-4135-9ed4-7b4e20f6e044n%40googlegroups.com.

[Greg Ercolano]

On 9/2/21 6:38 PM, Bill Spitzak wrote:

It does seem like a bug, calling value() should not have any side-effects. Especially because the tabs widget works even if nobody calls value().

    I think the problem here is with value(Fl_Widget*) which changes
    the current tab.

    What I've found is when you click on a tab, the Fl_Tabs::handle()
    changes the value of the tab using value(Fl_Widget*), which sets
    the new value, and returns 1 if it was changed or 0 if not.

    And based on that return value, handle() invokes do_callback()
    if a change was made.

    The problem is the value(Fl_Widget*) method is improperly returning
    0 even though the tab was indeed changed.

    This is because the logic in Fl_Tabs::value(Fl_Widget*) to detect a change
    assumes it can walk the loop calling hide() and show() on the children,
    and can check the visible() of the new tab as a way to detect if there was
    a change.

    The problem is when the loop calls hide() to deselect the first child, "Tab A",
    this single call to hide()..

        1. Sends "Tab A" an FL_HIDE event   -- OK
        2. Sends "Tab C" an FL_SHOW event -- THIS HERE IS THE PROBLEM
        3. Sends "Tab C" an FL_FOCUS event

    Step #2 causes Tab C to be shown before the loop expects it to be,
    and this confuses the loop's logic to detect that a change was made.

    Assuming that the above hide() event behavior is not a bug, this can
    I think be fixed/avoided by rewriting the value(Fl_Widget*) code this way:

 int Fl_Tabs::value(Fl_Widget *newvalue) {
+  Fl_Widget *oldvalue = value();
   Fl_Widget*const* a = array();
-  int ret = 0;
   for (int i=children(); i--;) {
     Fl_Widget* o = *a++;
     if (o == newvalue) {
-      if (!o->visible()) ret = 1;
       o->show();
     } else {
       o->hide();
     }
   }
-  return ret;
+  return oldvalue != newvalue ? 1 : 0;  // change detected
 }

    The only problem with that, and I think is the reason the code is the way it is,
    is that call to value() is non-trivial; it goes thru a loop just to find the current value(),
    so perhaps it was an efficiency call.

    But there really cant be very many tabs; there's a visual limit to how many
    there can be, so perhaps efficiency here is unwarranted.

[Greg Ercolano]

On 9/2/21 6:38 PM, Bill Spitzak wrote:

It does seem like a bug, calling value() should not have any side-effects. Especially because the tabs widget works even if nobody calls value().

    Ah, it is weird that just testing with Fl_Tabs::value(void) can apparently
    end up calling hide() and show().. yikes.

    That code goes back to 1999 though, so looks like you'd know for sure:

49a0693962 (Matthias Melcher   2006-08-17 13:43:07 +0000 308) Fl_Widget* Fl_Tabs::value() {
a7904da09a (Bill Spitzak       1999-10-15 09:01:48 +0000 309)   Fl_Widget* v = 0;
a7904da09a (Bill Spitzak       1999-10-15 09:01:48 +0000 310)   Fl_Widget*const* a = array();
a7904da09a (Bill Spitzak       1999-10-15 09:01:48 +0000 311)   for (int i=children(); i--;) {
a7904da09a (Bill Spitzak       1999-10-15 09:01:48 +0000 312)     Fl_Widget* o = *a++;
a7904da09a (Bill Spitzak       1999-10-15 09:01:48 +0000 313)     if (v) o->hide();
a7904da09a (Bill Spitzak       1999-10-15 09:01:48 +0000 314)     else if (o->visible()) v = o;
a7904da09a (Bill Spitzak       1999-10-15 09:01:48 +0000 315)     else if (!i) {o->show(); v = o;}
f9039b2ae2 (Michael R Sweet    1998-10-06 18:21:25 +0000 316)   }
f9039b2ae2 (Michael R Sweet    1998-10-06 18:21:25 +0000 317)   return v;
f9039b2ae2 (Michael R Sweet    1998-10-06 18:21:25 +0000 318) }

    Perhaps that's the cause, because that last line in the loop that
    calls o->show() on the last item seems to be maybe what's forcing
    the last child to be visible if no other one is..

[Greg Ercolano]

    ..and by that I mean the loop code in Fl_Tabs::value(Fl_Widget*) that changes
    the selected tab by hide()ing all but the one to be show()n
    gets confused when it reaches the last tab and finds it visible() before it show()s it,
    causing the loop to think there was no change when there in fact was.

    It would appear Fl_Widget* value(void) seems to enforce that:

        > Not more than one group is visible
        > That at least one group is visible (the last, if no others are)

    And that sounds like a good thing to enforce at least /somewhere/,
    as it's hard to imagine the Tabs widget with no group visible, and/or
    more than one group visible at a time.

    So perhaps the fix I suggested is the right one, assuming we want to
    leave the value(void) code alone, and the show() FL_HIDE/FL_SHOW event behavior
    is correct.

[Bill Spitzak]

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.

--

You received this message because you are subscribed to the Google Groups "fltk.general" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkgeneral...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/fltkgeneral/9a3b1031-913e-486b-2bef-c0ff991d4346%40seriss.com.

[Bill Spitzak]

Also means value() can return on the first visible child, which is slightly faster.

[Albrecht Schlosser]

We should really fix this because value() is non-const which is generally not to be expected by users. If it had been 'const' in the first place we wouldn't have had this user report at all!

I've been looking for a fix but we can't easily say which change would break existing user programs. Backwards compatibility might be an issue here.

As a first step I used Greg's patch but replaced his call of value() by a new method first_visible() [1] which is 'const' and does exactly that: it returns the first visible child or NULL (the name may be changed) if no child is visible. If we could instead make value(void) 'const' that would be the preferred solution though. This is the new (trivial) method:

// Note: Fl_Tabs::value() should be like this:
Fl_Widget* Fl_Tabs::first_visible() const {
  Fl_Widget*const* a = array();
  for (int i = children(); i--;) {
    Fl_Widget *o = *a++;
    if (o->visible())
      return o;
  }
  return 0;
}

Gregs patch becomes:

int Fl_Tabs::value(Fl_Widget *newvalue) {
  Fl_Widget*const* a = array();
  Fl_Widget *old = first_visible();

  for (int i=children(); i--;) {

    Fl_Widget* o = *a++;
    if (o == newvalue) {

      o->show();
    } else {
      o->hide();
    }
  }

  return (old != newvalue);
}


[1] @Greg: calling the existing value(void) method inside value(Fl_Widget *) could cause even more unexpected side effects than we're having now. Other than that: great findings, thanks!

... continued below ...

On 9/3/21 5:59 PM Bill Spitzak wrote:

Also means value() can return on the first visible child, which is slightly faster.

On Fri, Sep 3, 2021 at 8:58 AM Bill Spitzak <spi...@gmail.com> 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.

I agree that value() should not change states of children, I'd really like (and would expect!) value() to be a 'const' method.

The problem with this is that it's documented behavior and the user (program) can at any time add and delete children, show() or hide() them etc. etc.. Currently value() is called on the FL_SHOW event of the Fl_Tabs widget (which we could easily replace with another appropriate method), but we need a way to control the state after later user program actions (as mentioned above). IMHO the real problem is event delivery if none or more than one children are visible(), whereas drawing could always stop after the first child was drawn. The latter would be an easy change - if and (maybe) only if all children are drawn.

I was looking for a fix but was interrupted by other private stuff. I'm still looking for a better solution...

[Greg Ercolano]

On 9/3/21 9:43 AM, Albrecht Schlosser wrote:

[1] @Greg: calling the existing value(void) method inside value(Fl_Widget *) could cause even more unexpected side effects than we're having now. Other than that: great findings, thanks!

    I noticed value(void) is called elsewhere when you follow hide() down the  event rabbit hole.

    Looking quickly, it's called during the handling of e.g. FL_HIDE which is triggered
    by hide() and show() in value(Fl_Widget*). It's also called at the top of draw().

[Greg Ercolano]

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().

[Albrecht Schlosser]

Yeah, that doesn't make it better. I'll reply further on your next
message...

[Albrecht Schlosser]

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.

[Bill Spitzak]

I agree with most of this, it is the way to go. A few differences I think should be done:

1. value(Fl_Widget*) should do the recalc, rather than leaving it in a possible bad state until draw() calls recalc.

2. recalc might be better called "fix" or "make_exactly_one_tab_visible" or something, depending on what reads best, and it should be private (or maybe protected)

--
You received this message because you are subscribed to the Google Groups "fltk.general" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkgeneral...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/fltkgeneral/22d804de-0f92-a796-50d8-36e319b78d57%40online.de.

[Albrecht Schlosser]

On 9/3/21 9:50 PM Bill Spitzak wrote:

I agree with most of this, it is the way to go. A few differences I think should be done:

1. value(Fl_Widget*) should do the recalc, rather than leaving it in a possible bad state until draw() calls recalc.

My intention was to do this anyway if the given widget was a real child of the group (as documented). value(Fl_Widget*) did this already for all valid children. The only questionable case is (was) if the user calls it with a pointer that is not a valid child (or NULL). If we always wanted to leave the widget in a correct state this would imply that we selected the last child as the current child. So, if the user calls it with an invalid pointer: we "fix" this programming error. If the user calls it with NULL we call it a feature and select the last child.

I think this is plausible and could be documented this way.

2. recalc might be better called "fix" or "make_exactly_one_tab_visible" or something, depending on what reads best, and it should be private (or maybe protected)

I didn't say anything about accessibility yet. I agree that recalc (or whatever we call it) should be protected (not private). I'm sure someone will derive a widget from Fl_Tabs in the future and need the "recalc" method.

I'm open for suggestions of better names for this method. So far I used Greg's proposal for simplicity.

What about fix_visibility()? As a precedent we have Fl_Scroll::fix_scrollbar_order(), fl_fix_focus() and likely some more.

Thanks for your comments, Bill, I appreciate this very much.

[Albrecht Schlosser]

On 9/3/21 10:10 PM Albrecht Schlosser wrote:

On 9/3/21 9:50 PM Bill Spitzak wrote:

2. recalc might be better called "fix" or "make_exactly_one_tab_visible" or something, depending on what reads best, and it should be private (or maybe protected)

What about fix_visibility()? As a precedent we have Fl_Scroll::fix_scrollbar_order(), fl_fix_focus() and likely some more.

I propose (protected)
 

Fl_Widget *fix_current_tab(); // was: recalc()

 and I'm working with this one now until we decide to use another name.