dankamongmen / notcurses

blingful character graphics/TUI library. definitely not curses.
https://nick-black.com/dankwiki/index.php/Notcurses
Other
3.6k stars 112 forks source link

A mouse click can select a disabled item #2582

Closed kascote closed 2 years ago

kascote commented 2 years ago

After a mouse click and using ncmenu_mouse_selected, it returns the menu name without take into account the disabled state

Please include the following data:

dankamongmen commented 2 years ago

great find, thanks a lot for the report! i'll try to fix this up today for 3.0.6; it'll definitely be fixed by 3.1.0.

dankamongmen commented 2 years ago

alright, i'm looking into this using the menu PoC, and i'm unable to select any actual menu items with the mouse...hrm (xterm or kitty)

dankamongmen commented 2 years ago

i can select a top-level menu with the mouse, and it correctly does not select a disabled one...hrmm

dankamongmen commented 2 years ago

meanwhile, notcurses-demo seems to have no mouse capability....what the hell's going on?

kascote commented 2 years ago

the item is not selected, but the 'click' event came back from the get method, if you send the key event throw ncmenu_mouse_selected will return the item at that position, if the item is disabled must return null

dankamongmen commented 2 years ago

ok, i've got notcurses-demo fixed up for mouse menu events in master; we were throwing away any NCKEY_RELEASE. now let's see what elese is going on here.

kascote commented 2 years ago

for me this is not time sensitive or nothing.. prioritize as you will...

dankamongmen commented 2 years ago

nah, i'm looking into it now =] sorry it took even this long

dankamongmen commented 2 years ago

so it would seem that fixed notcurses-demo only for the notcurses-demo menu, not the help menu. so i suspect we have coordinate translation problems in play here, as well. probably need do a long hard look at how ncmenu handles mouse events, as i wrote this code back in the pre-1.0 days, and a great deal has changed since then.

dankamongmen commented 2 years ago

ok, looking at menu.c, we don't seem to handle subsection events there, just toplevel. That surprises me.

In that case, everyone must be supplying their own subsection handler...which yeah, makes sense, since it needs to run in our input context....right? Let's verify that...indeed, that's the whole point of ncmenu_mouse_selected().

ok, let me add that handler to the menu PoC, first off. it's already in notcurses-demo.

dankamongmen commented 2 years ago

i think we might possibly have introduced a regression when we moved to pixel-based mouseclicks? i've added it to menu, and successfully picked up Quit sometimes:

Y/X: 6/19
WIN Y/X: 6/19 dimy: 8
SELECTION: Quit

but other clicks that seemed to be there did not:

Y/X: 0/19
WIN Y/X: 0/19 dimy: 8
SECTION_X: 1
Y/X: 6/20
WIN Y/X: 6/20 dimy: 8
SELECTION: (null)
Y/X: 5/21
WIN Y/X: 5/21 dimy: 8
SELECTION: (null)
Y/X: 4/22
dankamongmen commented 2 years ago

ok so three problems noticed while adding this to menu:

and finally, as @kascote originally noted, i don't think ncmenu_mouse_selected() prunes out disabled items.

there's quite a lot of brokenness here!

dankamongmen commented 2 years ago

heh see #2606, ugh

dankamongmen commented 2 years ago

ok the menu PoC now supports mouse events for menu items; i'm going to resolve the problems listed above

dankamongmen commented 2 years ago

menus should open to the first non-disabled item, also. if you disable the first item, it's still selected upon first opening.

dankamongmen commented 2 years ago

menus should open to the first non-disabled item, also. if you disable the first item, it's still selected upon first opening.

this is done

dankamongmen commented 2 years ago

ok we no longer return disabled items from ncmenu_mouse_selected(), which ought resolve the original point of this bug @kascote .

dankamongmen commented 2 years ago

ok the horizontal restrictions are due to use of section_x() in ncmenu_mouse_selection(), and indeed, we have this comment:

  // FIXME section_x() works only off the section header lengths, meaning that                                                      
  // if we click an item outside of those columns covered by the header, it will                                                    
  // read as a -1 from section_x(). we want to instead get the unrolled section,                                                    
  // find its boundaries, and verify that we are within them. 
dankamongmen commented 2 years ago

we ought just be able to use these, no?

  int xoff;               // column offset from beginning of menu bar                                                               
  int bodycols;           // column width of longest item    
dankamongmen commented 2 years ago

alright, we now accept ncmenu mouse clicks through the proper horizontal range

dankamongmen commented 2 years ago

alright, we now pick up the correct item for mouseclicks on bottom menus

dankamongmen commented 2 years ago

ok, all this crap seems to work now.