Opened 12 years ago

Closed 10 years ago

Last modified 7 years ago

#6773 closed enhancement (fixed)

Menu: add timeout so submenu doesn't close if mouse touches parent node's sibling

Reported by: smith@… Owned by: Douglas Hays
Priority: low Milestone: 1.4
Component: Dijit Version: 1.1.0
Keywords: Cc: bill, Pete Smith
Blocked By: Blocking:

Description (last modified by bill)

Add fudge factor for popup. If you click a popupmenu(submenu) and then diagonally slide your mouse, there should be a small timeout to allow you to get into the submenu without it disappearing. If you try the "history" menu in firefox, hover over "recently closed tabs" and sort of go down and to the right to get into the large list. It hangs out for a second, allowing you to get in there. Dojo menus go down unless you keep your mouse entirely in the parent menu item the whole time. This makes for a squirrely menu.

Attachments (3)

6773.patch (3.4 KB) - added by Douglas Hays 10 years ago.
added delayed/cancellable menu close. Removed duplicate onClose() from _onBlur
6773.2.patch (4.7 KB) - added by Douglas Hays 10 years ago.
refreshed patch to fix problems that bill found initially
6773.3.patch (41.2 KB) - added by Douglas Hays 10 years ago.
updated patch to add robot tests and to fix #8624. Also adds ability to close MenuBar? menus with reclick of the menu item.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 12 years ago by guest

I just found another bug, click the little graphic (>) in a popupmenu item you will get an error

popup has no properties http://localhost/gq_smith/apache/web/GQ/js/dojo-1.1.1/dojo/dojo.js Line 322

comment:2 Changed 12 years ago by bill

Cc: smith@… removed
Milestone: 1.4
Reporter: changed from guest to smith@…

I agree with (1), but that occurs because we don't have a MenuBar widget yet, see #4634 (so you've basically duplicated that ticket).

I guess (2) makes sense. Whether it should be a timer or react based on distance is debatable; I guess either is OK and the timer is probably easier.

I split off the other bug you found as #6778.

comment:3 Changed 12 years ago by bill

Description: modified (diff)
Milestone: 1.42.0
Priority: highlow
Summary: Menu and Toolbar Behaviors not StandardMenu: add timeout so submenu doesn't close if mouse touches parent node's sibling

Updating title and comment to remove the comment about !Toolbar vs. Menu since that's what #4634 is about.

comment:4 Changed 11 years ago by guest

bill I would like to suggest that this isn't as low as you think - menus have an expected behavior, and the dijit menus are squirrely and hard to use. I have seen many demos of our software where people struggle with the precision needed to operate it. Certain things just have to be right, basic, core things.

comment:5 Changed 11 years ago by alex

I'm inclined to agree that this isn't low priority.

comment:6 Changed 11 years ago by Pete Smith

this still exists if you have two submenus on top of each other. case:

Menu

Submenu 1 > Submenu 2 >

Path: Hover over submenu 1, and then go diagonally down and to the right in an attempt to get into submenu 2. If you don't traverse DIRECTLY to the right (through submenu 1 ) and into submenu 2, 1 gets knocked down. For an example of how this should work, go into FF3's Bookmarks menu, hover over recently bookmarked, and then mouse down and to the right to get into the menu (skirting Recent Tags).

Note: this might seem minor, but I have seen in our product demos users struggle each time to use the top submenu (which of course is the most important). It makes for a squirrely menu.

comment:7 Changed 11 years ago by Pete Smith

for above, menu is

MENU
  Submenu 1 >
  Submenu 2 >

comment:8 Changed 11 years ago by Adam Peller

Milestone: 2.0tbd

httpete has asked that we re-triage this. Given that we have no 2.0 plans, is this something we should consider for 1.4 or 1.5? Is it still low priority?

comment:9 Changed 11 years ago by Pete Smith

Can we sneak this in for 1.4 gents? I say so because it affects a core foundation dijit's usability, every time I see a demo of our product the presenter suffers trying to choose the menu when the two are stacked.

Changed 10 years ago by Douglas Hays

Attachment: 6773.patch added

added delayed/cancellable menu close. Removed duplicate onClose() from _onBlur

comment:10 Changed 10 years ago by Douglas Hays

Cc: bill Pete Smith added

bill, httpate, please review the attached patch. I left a console.debug statement in to show the onClose calls since they were being duplicated before, and are now being delayed to demonstrate that none are being dropped.

comment:11 Changed 10 years ago by Douglas Hays

The patch tries to mimic the behavior of the Windows Start menu that allows for "diagonal" mouse movement.

comment:12 Changed 10 years ago by bill

Cool, glad you are working on this. I'm not sure I understand all the changes yet but I did try it out. I see two issues in test_Menu.html:

  1. moving diagonally from "Different popup" to the ColorPalette doesn't work right; the palette closes.
  2. if you open a MenuBar menu (like "View") and then click on a blank part of the screen, the menu doesn't close.

Changed 10 years ago by Douglas Hays

Attachment: 6773.2.patch added

refreshed patch to fix problems that bill found initially

comment:13 Changed 10 years ago by Douglas Hays

Milestone: tbd1.4
Owner: set to Douglas Hays

I refreshed the patch file to monitor popup hovering instead of item hovering since the ColorPallete? widget does not fire hover events like the MenuItem? widgets do. Also cleaned up the MenuBar? blur/close code. onClose on the MenuBar? was incorrectly being called that closed the dropdown menus almost as a side-effect. Now MenuBar? closes the child menus directly in _onBlur to prevent false onClose events.

comment:14 Changed 10 years ago by bill

Cool, that fixes the ColorPalette problem. There's still an issue with blurring though: you fixed MenuBar but not horizontal static menus, try clicking a blank area of the screen to close a popup from the left side menu in test_Menu.html.

Also, could you add to the menu robot tests for the diagonal movement, and (if it's not already there) for the thing about closing a MenuBar/vertical !Menu by clicking a blank area of the screen?

Oh there's a typo in this comment in the patch: "if the menu hovers over a menu popup", first "menu" should be "mouse", right?

Changed 10 years ago by Douglas Hays

Attachment: 6773.3.patch added

updated patch to add robot tests and to fix #8624. Also adds ability to close MenuBar? menus with reclick of the menu item.

comment:15 Changed 10 years ago by Douglas Hays

bill, httpete, please review/test as needed. There's quite a few menu changes.

comment:16 Changed 10 years ago by bill

Type: defectenhancement

Looks good. I was surprised the MenuBar and vertical navigation menus should act differently when clicking a menu item that's already open, but since you've tried it on other apps I'll take your word for it.

It seems like maybe you fixed #9846 with this patch too? If so please close it too when you check in.

Tests:

The menu robot tests are running fine for me on IE6.

On FF3.5/win I'm getting a bunch of errors in Menu_a11y.html though. Do you see those?

On Safari4, Menu_a11y.html crashes the browser, which is worse than before when it just got test failures. On mac it crashes during the test, where on windows it gets an error after the test when you start to click the browser menu.

Code:

About the code, I looked over the code changes, they seem pretty good but there's one difference that I noticed. A MenuBar etc. is no longer marked as "active" when the user tabs into it. As the comment in _onFocus() says, a Menu should be marked as active by:

  1. clicking it
  2. tabbing into it
  3. being opened by a parent menu

I'm not sure what the practical effects of that change are. It breaks customized CSS where active vs. passive menus have different styles, but maybe there are more serious issues too.

BTW, since you've commented out onFocus (presumably to erase it), can you preserve that comment above somewhere, since it sort-of defines what "active" means?

comment:17 Changed 10 years ago by Douglas Hays

Resolution: fixed
Status: newclosed

(In [20098]) Fixes #6773, #8624, #9846 !strict. Added ESC keydown handling in the popup manager for WebKit? so menus can be closed with the keyboard. Added a pending-close state to menus so if the mouse touches something else but moves back to a target menu, then the close is cancelled. Added ability to close MenuBar? menu's by reclicking the menubar item to be consistent with other applications. Collpased multiple deferred focus events to prevent incorrect blurring. Added numerous automated tests to demonstrate changed behavior. All tests ran on WIndows. Mac is currently experiencing robot issues. Removed the Hover CSS style if the keybaord is used to select a different item in the same menu after moving the mouse to hover a slibling item to prevent multiple menu selections. Removed isActive after a menu item is selected or on a menu blur event so the menu doesn't popup again on subsequent hovering. Removed _onFocus handling from Menu since it can fire after a menu has closed and now isActive is set from onItemClick.

comment:18 Changed 10 years ago by Pete Smith

Gents,

This seems to be much smoother in v 1.4b1. I don't know if what I am seeing is an artifact of this fix, or some other change. But when I click a menu, or display a dialog, I get a flash of my page ever so quickly, I can see my logo (in the upper left) appear right before the menu / dialog appears. I will research this but wanted to make an note here.

comment:19 Changed 10 years ago by bill

Yes, I see you filed #10111 about that problem, it's actually from #10041 (and is unrelated to the timeout changes).

comment:20 Changed 7 years ago by bill

See also #16867.

Note: See TracTickets for help on using tickets.