STR #2950

FLTK matrix user chat room
(using Element browser app)   FLTK gitter user chat room   GitHub FLTK Project   FLTK News RSS Feed  
  FLTK Apps      FLTK Library      Forums      Links     Login 
 Home  |  Articles & FAQs  |  Bugs & Features  |  Documentation  |  Download  |  Screenshots  ]
 

Return to Bugs & Features | Roadmap 1.3 | Post Text | Post File | Prev | Next ]

STR #2950

Application:FLTK Library
Status:5 - New
Priority:1 - Request for Enhancement, e.g. asking for a feature
Scope:3 - Applies to all machines and operating systems
Subsystem:Core Library
Summary:Menu Item behaviour
Version:1.3-feature
Created By:rokan2
Assigned To:Unassigned
Fix Version:Unassigned
Update Notification:

Receive EMails Don't Receive EMails

Trouble Report Files:

Post File ]

No files


Trouble Report Comments:

Post Text ]
Name/Time/Date Text top right image
 
#1 rokan2
09:14 Apr 11, 2013
Currently clicking on ANY menu item causes the whole menu to close - which is sometimes annoying, even if the item is:

1) A submenu. Usually submenu is just an entry point and "mouse hovering" opens the submenu automatically, an accidental clicking on this entry point should not
close the whole menu. "Not closing" the menu would also indicate that no action was performed and choosing particular submenu-item is required to invoke an action

2)  A checkbox or radio box. If menu item group is set of check-boxes and user wants to change a few settings at the same time, he/she needs to reopen the menu for each single item change. Sometimes user re-opens the menu even for single checkbox just to visually confirm that the state was changed. In such a case they expect that menu closes only if you either hit "Esc" or click menu-opening widget (menu-bar or menu-button)

Following is an implementation (just included in this message for simplicity) to give the developper fine control. It is very conservative (does not change current behaviour) but adds the programmer possibility to control when the items close the menus.

The API consists of two additional functions, they are implementad as "static" of Fl_Menu_Item namespace:

class Fl_Menu_Item {
  ...
public:
  static void no_close_flags(int f){no_close_flags_ = f;}
  static int no_close_flags(){return no_close_flags_;}
private:
  static int no_close_flags_;
};

The user can globally change how when the items do not close the menus - ie at the beginning of the program:

Fl_Menu_Item::no_close_flags(FL_MENU_TOGGLE|FL_MENU_RADIO|FL_SUBMENU|FL_SUBMENU_
POINTER);

User can also control the items individually like:

  #define FL_MENU_NO_CLOSE 0x200
  Fl_Menu_Item::no_close_flags(FL_MENU_NO_CLOSE);

and then OR this custom flag to the other flags of particular menu items.


Implementation:

1) In file "Fl_Menu_Item.H" define the functions as rescribed above


2) In file "Fl_Menu.cxx" change code in function menuwindow::early_hide_handle(int e), line approx. 783:

  #if 1 // uncommenting - makes defined items leave the menu up
      const Fl_Menu_Item* m = pp.current_item;
      if (m && button && (m->flags & Fl_Menu_Item::no_close_flags())) {
        ((Fl_Menu_*)button)->picked(m);
        pp.p[pp.menu_number]->redraw();
      } else
  #endif


3) In file "Fl_Menu_.cxx" implement

      int Fl_Menu_Item::no_close_flags_ = 0;

   and change code in function Fl_Menu_::picked() from line approx 271:

        if (value_ && value_->callback_)
           value_->do_callback((Fl_Widget*)this);
        else if(!(value_->flags & Fl_Menu_Item::no_close_flags()& (FL_SUBMENU | FL_SUBMENU_POINTER)))
          // do not execute parent's widget (common) callback if there is no direct menuitem callback assigned
          // and menu is not closed and isem is just a submenu (no valid item picked)
          do_callback();
         }


The code was tested and everything works as expected.

Roman
 
 
#2 greg.ercolano
11:08 Apr 11, 2013
We can probably avoid breaking ABI if we implement this new bitflag
as part of the existing flags, eg:

  enum { // values for flags:
    :
    FL_MENU_NOCLOSE  = 0x200  // Don't close menu if submenu|checkbox clicked
  };

Unless I'm missing something, is there a reason to avoid this?
 
 
#3 greg.ercolano
11:17 Apr 11, 2013
Oh, I see, you are suggesting this, /and/ a global flag to affect
the entire menu, yes.

I guess we can't avoid breaking ABI for the global flag, which
is probably best implemented in Fl_Menu_ by adding an int to the
class, protected by our new ABI macro so it can be used during
patch releases)
 
 
#4 rokan2
12:01 Apr 11, 2013
I have put the functions in Fl_Menu_Item namespace only because maming them that way seems to make sense. As far as I know, static functions (and static class variables) do not break ABI: they are not "really" class members, just additional ordinary functions and global variables with fancy naming sugar (functions with class namespace scope). For that matter even addition of a non-virtual class function should not break ABI either - any compiler guru can confirm this or am I wrong?

Anyway, the function is to set a mask which menu-item flags should indicate not to close the menu. In that way an existing application could take advantage of new behaviour automatically (especially those pesky submenu items) just by adding simple
 Fl_Menu_Item::no_close_flags(FL_MENU_TOGGLE|FL_MENU_RADIO|FL_SUBMENU|FL_SUBMENU_
POINTER);

or whatever flags you want at the beginning of the program. But if the user wants old behaviour and set only a few items explicitly, he can always define his own flag and set Fl_Menu_Item::no_close_flags(...) mask to this custom flag only.

R.
 
 
#5 AlbrechtS
02:53 Apr 12, 2013
Roman, this looks like a good idea, I recommend adding it, however with two caveats. I'll post them in followup messages.

WRT ABI issues: adding flags and static methods doesn't break the ABI, so we should be safe here.
 
 
#6 AlbrechtS
02:57 Apr 12, 2013
#1: I don't like the idea to encourage users to add custom flags in such central structures like Fl_Menu_Items, as proposed with
#define FL_MENU_NO_CLOSE 0x200.

However, we *should* add this to the menu item flags, as Greg proposed, and use it unconditionally everywhere in the menu code. Since this is a pure addition, this is a good idea anyway and won't break existing code.

There's no need to include this one in the global no_close_flags(), since it would work simply by adding it to any Fl_Menu_Item, as the user/programmer likes.
 
 
#7 AlbrechtS
03:16 Apr 12, 2013
#2: I believe that the following part of this proposal should not be added as given:

"change code in function Fl_Menu_::picked() from line approx 271:
 ...
        else if(!(value_->flags & Fl_Menu_Item::no_close_flags()& (FL_SUBMENU | FL_SUBMENU_POINTER)))
          // do not execute parent's widget (common) callback if there is no direct menuitem callback assigned
          // and menu is not closed and isem is just a submenu (no valid item picked)
          do_callback();
         }
"

It think that not calling a callback for submenus is unrelated to the option not to close the menu when the item is clicked on. What if someone wants a callback when a submenu is clicked on, but doesn't want the menu to be closed? Setting the FL_MENU_NOCLOSE flag would prevent the callback from being called in this situation, if I understand correctly what is going on here. I don't really know why one would want a callback to be called when a submenu gets selected, but I can imagine that you could change the submenu before it is popped up (or popup()ed? ;-)).

Also, I believe the code could be changed to be more clear, but maybe you did it as proposed because you had compatibility in mind. Wouldn't something like this be better?

if ( !((value_->flags & Fl_Menu_Item::no_close_flags()) && (FL_SUBMENU | FL_SUBMENU_POINTER)) )

Well, I really don't grok it entirely now, but I hope you got my idea. This should be discussed further, and maybe a test programm would be fine. Roman, could you post one and describe a procedure to test what you want to achieve, so that we can see the effect when implementing it?
 
 
#8 rokan2
06:52 Apr 12, 2013
Hi Albrecht,
 
1) To have global function (although as static in  Fl_Menu_Item) no_close_flags(int f) is to allow user simply to change begaviour globally for isntance for all submenus by calling

  Fl_Menu_Item::no_close_flags(FL_SUBMENU|FL_SUBMENU_POINTER);

Then it woult not be required to set any menuitem flag explicitly, so all code (for instance fluid generated) would work with new behaviour as expected

2) I an not against adding an extra FL_MENU_NOCLOSE enumeration flag, just showing that this is unnecessary as it can be done by the user. Having function no_close_flags() allows to control behaviour BOTH globaly and in per-item fasion. But adding FL_MENU_NOCLOSE to api and setting by default 

   static int no_close_flags_ = FL_MENU_NOCLOSE;

or even to

  static int no_close_flags_ = FL_MENU_NOCLOSE | FL_SUBMENU | FL_SUBMENU_POINTER;
      // breaks backward compatibility

is fine with me, although the second one breaks backward compatibility

3)
Your code
  if ( !((value_->flags & Fl_Menu_Item::no_close_flags()) && (FL_SUBMENU | FL_SUBMENU_POINTER)) )


does not make sense to me - second part  && (FL_SUBMENU | FL_SUBMENU_POINTER) is a constant expression and is always true.

The code change as proposed is to assure strict backward compatibility: submenus until now never called "widget" common callback (even when menu is closed) with submenu-item picked. This is to avoid calling this callback with wrong item picked - which can not be selected for instance Fl_Choice with submenu (you can not set this Fl_Choice to the state with submenu-item picked). However new code would indicate "submenu-item" as picked during callback and could even crash a program if the program relies that only ordinary items can be selected. The user still can assign a callbback directly to submenu-item if he wishes so (probably a rare case).

And yes, this is a real-life case: without the change one of my applications with Fl_Choice with submenus crashed.

3) I would propose to have functions no_close_flags() NOT inlined to avoid initialization fiasco (if somebody would call this before main() or some crazy pal would call gui before main()), I would implement them in Fl_Menu.cxx as

static int menu_no_close_flags_ = FL_MENU_NOCLOSE;
void Fl_Meu_Item::no_close_flags(int m){return menu_no_close_flags_ = m;}
int Fl_Meu_Item::no_close_flags(){return menu_no_close_flags_;}

Roman
 
 
#9 rokan2
07:08 Apr 12, 2013
Ops, the second line from the bottom should be of course without return:
void Fl_Meu_Item::no_close_flags(int m){menu_no_close_flags_ = m;}
 
 
#10 AlbrechtS
10:19 Apr 12, 2013
Hi Roman,

1) I didn't write that I didn't want that global flag, and I understood what you wanted to achieve with it. That's all okay so far.

2) It was not the point that adding a new flag is "unnecessary as it can be done by the user". My point is: we should *avoid* to make the user set any flags in the menu items except those that are defined in the library. One argument is to be free to define other bits for new use cases within the library w/o risking damage if a user defined exactly this bit for his purposes (as it would be possible with your primary proposal). Therefore I am for adding this extra bit to the enumeration within the library. Only that was the reason to do it like Greg proposed, everything else just like your proposal would be okay with me.

3) Yes, you're right, my code was bad - I was in a hurry. But I still believe that the callback issue is another issue unrelated to the no_close flag, so this shouldn't be mixed. However, I don't have the time to think further about it now, so I'll post later regarding this.

4) to your new code proposals: okay with one exception: if we use the FL_MENU_NOCLOSE flag, then it would be handy to change this one:

void Fl_Meu_Item::no_close_flags(int m){menu_no_close_flags_ = m|FL_MENU_NOCLOSE;}

so that FL_MENU_NOCLOSE will always be set in the flags for testing.
 
 
#11 ianmacarthur
16:21 Sep 04, 2014
Do we want to go ahead with this, or...?  
bottom left image   bottom right image

Return to Bugs & Features | Post Text | Post File ]

 
 

Comments are owned by the poster. All other content is copyright 1998-2022 by Bill Spitzak and others. This project is hosted by The FLTK Team. Please report site problems to 'erco@seriss.com'.