Skip to content

Commit d81479b

Browse files
author
Steven G. Harms
committed
menubar: implement jQuery UI consistency
This code base had been noted for being "unusual" in comparison with other widgets e.g. `dialog` by @scottgonzalez et al. Further drift was introduced by @sgharms. This commit reflects the work of bringing `menubar` closer to `dialog` by means of removing extremely unusual code invocations and by implementing feedback from a detailed review by @jzaefferer.
1 parent d89437f commit d81479b

File tree

3 files changed

+305
-427
lines changed

3 files changed

+305
-427
lines changed

tests/unit/menubar/menubar_core.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,18 @@ test( "Cursor keys should move the focus", function() {
3131
ok( !firstMenuItem.hasClass( "ui-state-focus" ), "RIGHT should move focus off of focused item" );
3232
$( document.activeElement ).simulate( "keydown", { keyCode: $.ui.keyCode.LEFT } );
3333
equal( document.activeElement, firstMenuItem[0], "LEFT should return focus first menuItem" );
34-
} );
34+
});
35+
36+
test ( "_destroy should successfully unwrap 'span.ui-button-text' elements" , function() {
37+
expect(1);
38+
39+
var containedButtonTextSpans,
40+
element = $( "#bar1" ).menubar();
41+
42+
element.menubar( "destroy" );
43+
containedButtonTextSpans = element.find( "span.ui-button-text" ).length
44+
equal( containedButtonTextSpans, 0, "All 'span.ui-button-text' should be removed by destroy" );
45+
});
46+
3547

3648
})( jQuery );

tests/unit/menubar/menubar_events.js

Lines changed: 12 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -43,75 +43,27 @@ test( "hover over a menu item with no sub-menu should close open menu", function
4343
menuItemWithDropdown.trigger("click");
4444
menuItemWithoutDropdown.trigger("click");
4545

46-
equal($(".ui-menu:visible").length, 0, "After triggering a sub-menu, a click on a peer menu item should close the opened sub-menu");
46+
equal($(".ui-menu:visible").length, 0, "After triggering a sub-menu, a click on a peer menu item should close the opened sub-menu");
4747
});
4848

49-
test ( "_findNextFocusableTarget should find one and only one item", function() {
50-
expect(2);
51-
52-
var element = $("#bar1").menubar(),
53-
menubarWidget = element.data( "ui-menubar" ),
54-
firstMenuItem = $( "#bar1 .ui-menubar-item" ).eq( 0 ),
55-
expectedFocusableTarget = $("#bar1 .ui-menubar-item .ui-widget").eq( 0 ),
56-
result = menubarWidget._findNextFocusableTarget( firstMenuItem );
57-
58-
equal( expectedFocusableTarget[0], result[0], "_findNextFocusableTarget should return the focusable element underneath the menuItem" );
59-
equal( 1, result.length, "One and only one item should be returned." );
60-
});
61-
62-
asyncTest( "TAB order should be sane mirroring dialog's test", function() {
63-
expect( 3 );
49+
test( "Cursor keys should move focus within the menu items", function() {
50+
expect( 6 );
6451

6552
var element = $( "#bar1" ).menubar(),
66-
firstMenuItem = $( "#bar1 .ui-menubar-item .ui-button:first" );
67-
68-
function checkTab() {
69-
setTimeout( start );
70-
ok( !firstMenuItem.hasClass( "ui-state-focus" ), "The manually focused item should no longer have focus after TAB" );
71-
//setTimeout( start );
72-
}
53+
firstMenuItem = $( "#bar1 .ui-menubar-item .ui-button:first" ),
54+
nextLeftwardMenuElement = firstMenuItem.parent().siblings().last().children().eq( 0 );
7355

56+
equal( element.find( ":tabbable" ).length, 1, "A Menubar should have 1 tabbable element on init." );
7457
firstMenuItem[ 0 ].focus();
75-
ok( $( firstMenuItem ).hasClass( "ui-state-focus" ), "Should have focus class" );
76-
77-
setTimeout(function() {
78-
equal( document.activeElement, firstMenuItem[0], "Focus set on first menuItem" );
79-
$( document.activeElement ).simulate( "keydown", { keyCode: $.ui.keyCode.TAB } );
80-
setTimeout( checkTab );
81-
})
82-
83-
} );
84-
85-
asyncTest( "TAB order should be sane", function() {
86-
expect( 3 );
87-
88-
89-
var element = $( "#bar1" ).menubar(),
90-
debugDelay = 0,
91-
firstMenuItem = $( "#bar1 .ui-menubar-item .ui-button:first" );
92-
93-
/* Make the qunit fixture visible if we're debugging this test*/
94-
if ( debugDelay ) {
95-
$('<link rel="stylesheet" href="../../../themes/base/jquery.ui.all.css" />').appendTo("head");
96-
$( "#qunit-fixture" ).css({ right: "300px", top: "300px", left:0 });
97-
}
98-
99-
setTimeout(function(){
100-
firstMenuItem[ 0 ].focus();
10158

102-
function postFocus(){
103-
ok( !firstMenuItem.hasClass( "ui-state-focus" ), "The manually focused item should no longer have focus after TAB" );
104-
setTimeout( start );
105-
};
59+
ok( firstMenuItem.hasClass( "ui-state-focus" ), "After a focus event, the first element should have the focus class." );
60+
$( document.activeElement ).simulate( "keydown", { keyCode: $.ui.keyCode.LEFT } );
10661

107-
setTimeout(function() {
108-
ok( firstMenuItem.hasClass( "ui-state-focus" ), "Should have focus class" );
109-
equal( document.activeElement, firstMenuItem, "Focus set on first menuItem" );
110-
$( document.activeElement ).simulate( "keydown", { keyCode: $.ui.keyCode.TAB } );
111-
setTimeout( postFocus );
112-
});
11362

114-
}, debugDelay );
63+
ok( !firstMenuItem.hasClass( "ui-state-focus" ), "After a keypress event, the first element, should no longer have the focus class." );
64+
ok( nextLeftwardMenuElement.hasClass( "ui-state-focus" ), "After a LEFT cursor event from the first element, the last element should have focus." );
65+
equal( element.find( ":tabbable" ).length, 1, "A Menubar, after a cursor key action, should have 1 tabbable." );
66+
equal( element.find( ":tabbable" )[ 0 ], nextLeftwardMenuElement[ 0 ], "A Menubar, after a cursor key action, should have 1 tabbable." );
11567

11668
} );
11769

0 commit comments

Comments
 (0)