[jquery-ui] r3424 committed - menu: nested menu refactoring, for now as its own widget
codesite...@google.com
Author: joern.zaefferer
Date: Mon Nov 9 05:11:21 2009
Log: menu: nested menu refactoring, for now as its own widget
http://code.google.com/p/jquery-ui/source/detail?r=3424
Modified:
/branches/dev/tests/visual/menu/nested.html
=======================================
--- /branches/dev/tests/visual/menu/nested.html Mon Nov 9 03:10:32 2009
+++ /branches/dev/tests/visual/menu/nested.html Mon Nov 9 05:11:21 2009
@@ -9,67 +9,98 @@
<script type="text/javascript"
src="../../../ui/jquery.ui.core.js"></script>
<script type="text/javascript"
src="../../../ui/jquery.ui.menu.js"></script>
<script type="text/javascript"
src="../../../ui/jquery.ui.position.js"></script>
- <script type="text/javascript"
src="http://jqueryui.com/themeroller/themeswitchertool/"></script>
<script type="text/javascript">
$(function() {
- $.fn.themeswitcher && $('<div/>').css({
- position: "absolute",
- right: 10,
- top: 10
- }).appendTo(document.body).themeswitcher();
-
-
- var submenu;
- var menu = $("#menu").menu({
+ $.widget("ui.nestedmenu", {
+ _init: function() {
+ var self = this;
+ this.active = this.element;
+
+ // hide submenus and create indicator icons
+ this.element.find("ul").hide().prev("a").prepend('<span class="ui-icon
ui-icon-carat-1-e"></span>');
+
+ this.element.find("ul").andSelf().menu({
+ selected: this.options.selected,
+ focus: function(event, ui) {
+ self.active = ui.item.parent();
+ // put a previous submenu back into its place and hide it
+ self.hideDown();
+ var nested = $(">ul", ui.item);
+ if (nested.length) {
+ // append to body in order to display the submenu above the parent
menu, instead of inside of it
+ nested.appendTo(document.body).menu("deactivate").show().position({
+ my: "left top",
+ at: "right top",
+ of: ui.item
+ // store the current submenu
+ }).data("menuparent", ui.item);
+
+ self.active.data("submenu", nested);
+ }
+ }
+ })
+ },
+
+ up: function() {
+ if (!this.active.data("menuparent"))
+ return;
+ this.active.menu("deactivate");
+ this.active = this.active.data("menuparent").parent();
+ },
+
+ down: function() {
+ var submenu = this.active.data("submenu");
+ if (!submenu)
+ return;
+ submenu.data("menu").activate(submenu.children(":first"))
+ this.active = submenu;
+ },
+
+ show: function() {
+ this.element.menu("deactivate").show();
+ this.active = this.element;
+ },
+
+ hide: function() {
+ this.hideDown();
+ var child = this.active.hide(), parent;
+ while(child.data("menuparent")) {
+ parent = child.data("menuparent");
+ child.appendTo(parent).removeData("menuparent");
+ child = parent.parent().removeData("submenu").hide();
+ }
+ },
+
+ hideDown: function() {
+ var submenu = this.active.data("submenu");
+ while(submenu) {
+ var parent = submenu.data("menuparent");
+ submenu.appendTo(parent).hide().removeData("menuparent");
+ parent.parent().removeData("submenu");
+ submenu = submenu.data("submenu");
+ };
+ }
+ });
+
+ var nestedmenu = $("#menu").nestedmenu({
selected: function(event, ui) {
$("#log").append("<div>Selected " + ui.item.text() + "</div>");
- },
- focus: function(event, ui) {
- // put a previous submenu back into its place and hide it
- if (submenu) {
- submenu.appendTo(submenu.data("menuparent")).hide();
- submenu = null;
- }
- var nested = $("ul", ui.item);
- if (nested.length) {
- nested.menu({
- selected: function(event, ui) {
- $("#log").append("<div>Selected " + ui.item.text() + "</div>");
- }
- // append to body in order to display the submenu above the parent
menu, instead of inside of it
- }).appendTo(document.body).menu("deactivate").show().position({
- my: "left top",
- at: "right top",
- of: ui.item
- });
- // store the current submenu
- submenu = nested;
- submenu.data("menuparent", ui.item);
- }
}
}).hide();
- // hide submenus and create indicator icons
- menu.find("ul").hide().prev("a").prepend('<span class="ui-icon
ui-icon-carat-1-e"></span>');
-
-
$("button").click(function(event) {
// TODO required to prevent the click handler below from handling this
event
event.stopPropagation();
- $("#menu").menu("deactivate").show().position({
+ nestedmenu.nestedmenu("show").position({
my: "left top",
at: "right top",
of: event.pageX > 0 ? event : this
});
$().one("click", function() {
- menu.hide();
- if (submenu) {
- submenu.appendTo(submenu.data("menuparent")).hide();
- submenu = null;
- }
+ nestedmenu.nestedmenu("hide");
})
}).keydown(function(event) {
- var menu = submenu && submenu.data("menufocussed") &&
submenu.data("menu") || $("#menu" + this.id).data("menu");
+ var menu = nestedmenu.data("nestedmenu").active.data("menu");
if (menu.widget().is(":hidden"))
return;
event.stopPropagation();
@@ -84,17 +115,10 @@
menu.previous();
break;
case $.ui.keyCode.LEFT:
- if (submenu && submenu.data("menufocussed")) {
- submenu.data("menufocussed", false);
- menu.deactivate();
- }
+ nestedmenu.nestedmenu("up");
break;
case $.ui.keyCode.RIGHT:
- if (submenu) {
- submenu.data("menufocussed", true);
- menu = submenu.data("menu");
- menu.activate(menu.element.children(":first"));
- }
+ nestedmenu.nestedmenu("down");
break;
case $.ui.keyCode.DOWN:
menu.next();
@@ -103,25 +127,11 @@
case $.ui.keyCode.ENTER:
case $.ui.keyCode.TAB:
menu.select();
- menu.widget().hide();
- if (submenu) {
- submenu.appendTo(submenu.data("menuparent"));
- submenu.removeData("menuparent");
- submenu = null;
- // also hide parent
- $("#menu" + this.id).hide();
- }
+ nestedmenu.nestedmenu("hide");
event.preventDefault();
break;
case $.ui.keyCode.ESCAPE:
- menu.widget().hide();
- if (submenu) {
- submenu.appendTo(submenu.data("menuparent"));
- submenu.removeData("menuparent");
- submenu = null;
- // also hide parent
- $("#menu" + this.id).hide();
- }
+ nestedmenu.nestedmenu("hide");
break;
default:
clearTimeout(menu.filterTimer);
@@ -177,7 +187,23 @@
<ul>
<li><a href="#">Aberdeen</a></li>
<li><a href="#">Ada</a></li>
- <li><a href="#">Adamsville</a></li>
+ <li>
+ <a href="#">Adamsville</a>
+ <ul>
+ <li><a href="#">Anaheim</a></li>
+ <li>
+ <a href="#">Cologne</a>
+ <ul>
+ <li><a href="#">Mberdeen</a></li>
+ <li><a href="#">Mda</a></li>
+ <li><a href="#">Mdamsville</a></li>
+ <li><a href="#">Mddyston</a></li>
+ <li><a href="#">Mmesville</a></li>
+ </ul>
+ </li>
+ <li><a href="#">Frankfurt</a></li>
+ </ul>
+ </li>
<li><a href="#">Addyston</a></li>
<li><a href="#">Amesville</a></li>
</ul>
Jörn Zaefferer
Richard D. Worth
Jörn Zaefferer
Jörn
Todd Parker
Scott Jehl
Scott Jehl
Jörn Zaefferer
Currently the open-submenu behaviour is the same for mouse and keyboard, which I think works quite well. Its not how other menus work like, but I'd like to at least explore this approach. Is there any drawback to showing it on focus?
Jörn
Scott Jehl
Jörn Zaefferer
Jörn
Todd Parker
Scott Jehl
Scott Jehl
Jörn Zaefferer
Jörn
Scott Jehl
Jörn Zaefferer
Is the markup hierachy really important, as long as we provide the propery ARIA attributes? Are there any for nested lists?
Jörn
Scott Jehl
Jörn Zaefferer
Thats still different from what we have no, where activation just selects the menu item, not the submenu. Windows menus behave that way, so implementating that together with the aria-haspopup attribute should provide a good solution. I think I can extend the menu's focus-event to provide the information if the focus was triggered by mouse or keyboard.
Also worth reading is the ARIA example: http://www.w3.org/TR/wai-aria/#managingfocus
Especially this: "Sometimes a tree structure is not explicit via the DOM and logical structure of a page. In such cases the relationships must still be made explicit using the states and properties. In the following example, the
aria-owns attribute indicates that the div with id "external_listitem" should be considered a child of the ul element with the property, even though it is not a child in the DOM."In other words, even when picking the DOM apart for layout purposes, using the appropiate ARIA roles we can provide the tree relationship.
Am I missing something? If not, I'll go on and implement those attributes and change the focus-submenu behaviour. Once that is in place, I'll look into delay opening and closing submenus and build more examples.
Jörn
Scott Jehl
Jörn Zaefferer
Dunno what an "embedded hierachy" is supposed to be, just plain nested lists? What are alternative workarounds for the scrolling issue?
Jörn
Scott Jehl
Dunno what an "embedded hierachy" is supposed to be, just plain nested lists? What are alternative workarounds for the scrolling issue?
Kevin Dalman
ui.position -- I am using the version linked in this demo. I'm posting
it here because the nestedmenu widget demo shows the problem. Here is
what I did...
To make the menu align to the bottom/left of the trigger element (I'm
using a hyperlink instead of a button), I changed the alignment on the
initalization options...
*** ORIGINAL ***
my: "left top",
at: "right top",
of: event.pageX > 0 ? event : this
});
my: "left top",
at: "right top",
});
The orginal 'of' option only seems applicable if creating a context-
menu. If the menu is attached to a trigger-element, it *should*
specify 'this'.
As soon as I made this change, it exposed a bug in ui.position: The
position of the menus is *miscalculated* the first time they are
opened. After the first time each menu/submenu is displayed, the
position corrects itself.
I traced the glitch to 'this.offsetLeft', which was not returning the
correct value. I believe this is related to the fact that the menu is
a UL element. If you do not hide the menu mark-up onLoad, you'll
notice that it is not left-aligned to the margin. I assume the UL must
have a left-margin, which this.offsetLeft takes into consideration
when returning an offset value.
To address this issue, I replace this.offsetLeft with $(this).position
().left. Here is the original and revised markup (punctuation adjusted
to my standard)...
***
*** ui.position - ORIGINAL
***
var
offset = elem.offset()
, delta = {
left: parseInt(elem.css('left'), 10)
, top: parseInt(elem.css('top'), 10)
}
;
// in case of 'auto'
delta.left = !isNaN(delta.left)
? delta.left
: isRelative
? 0
: this.offsetLeft
;
delta.top = !isNaN(delta.top)
? delta.top
: isRelative
? 0
: this.offsetTop
;
***
*** ui.position - REVISED
***
var
offset = elem.offset()
, position = elem.position()
, delta = {
left: parseInt(elem.css('left'), 10)
, top: parseInt(elem.css('top'), 10)
}
;
// in case of 'auto'
delta.left = !isNaN(delta.left)
? delta.left
: isRelative
? 0
: position.left
;
delta.top = !isNaN(delta.top)
? delta.top
: isRelative
? 0
: position.top
;
Now the returned values are correct and the menus line up correctly
the first time they are accessed. Since position() is a core method,
it seems a reasonable solution to use this rather than the problematic
offsetLeft/Top.
FYI, this glitch appeared under IE7 with the HTML 4.01 Strict doctype.
I did not test any other variations.
It's easy to reproduce this misalignment - just change the options on
Jörn's demo page, ie:
nestedmenu.nestedmenu("show").position({
my: "left top",
at: "right top",
});
I'm going to use this for navigation menus in a production website.
I'll let you know if I stumble across any other issues.
/Kevin
On Nov 9, 5:22 am, Jörn Zaefferer <joern.zaeffe...@googlemail.com>
wrote:
>
> Give it a try:http://jquery-ui.googlecode.com/svn/branches/dev/tests/visual/menu/ne...
>
Scott González
The code you're talking about is in the .offset() setter, which will not be released. This code lives in core now (and is completely different from this code). The plugin can't be updated until we pull 1.4 into trunk though.
Kevin Dalman
> The code you're talking about is in the .offset() setter, which will not be
> released. This code lives in core now (and is completely different from this
> code).
dependancy on this new offset-setter functionality, and the comment
says it "is planned for jQuery 1.4", so I considered it an integral
part of the ui.position widget.
> The plugin can't be updated until we pull 1.4 into trunk though.
however it's useful.
Scott González
part in ui.postion? That should show if the bug will exist for real.
Thanks.
Kevin Dalman
offset code in ui.position...
There IS still a bug similar to my previous description, though the
position errors are slightly different. Now the menu appears:
* aligned to the top of the page
* shifted left by the amount of BODY.paddingLeft
As before, each menu/submenu displays in the correct position *after
the first time shown*.
I found the setOffest method in the nightly build and applied the same
fix as before. This fixed the problem - everything appears in the
correct position the first time. Here is the modified code:
// line 5062
var curElem = jQuery( elem ),
curOffset = curElem.offset(),
position = curElem.position(), // NEW
// OLD curTop = parseInt( jQuery.curCSS( elem, 'top', true ),
10 ) || 0,
// OLD curLeft = parseInt( jQuery.curCSS( elem, 'left', true ),
10) || 0,
props = {
top: (options.top - curOffset.top) + position.top, // OLD
curTop
left: (options.left - curOffset.left) + position.left // OLD
curLeft
};
It seems that position() handles 'hidden element' correctly, but curCSS
() does not.
Hope that helps.
Kevin Dalman
followed the convention of prefixing jQuery objects with "$". This
would make it easier to follow because "elem" would always be the DOM-
element and $elem the elem wrapped in a jQuery object. This would
avoid the different syntax in each area, like "curElem", which still
doesn't make clear whether it is a DOM or jQuery reference. (though
the setOffset method is so small that it's not a problem)
The same '$name' standard should apply to UI widgets. It makes object
pointers crystal clear. It also creates a clear relationship between
the DOM and jQuery objects, eg: $(menu) = $menu
Just my 2-cents.
>
> - Show quoted text -
Scott González
BTW, it would be nice if code in the core and in all jQuery libraries
followed the convention of prefixing jQuery objects with "$". This
would make it easier to follow because "elem" would always be the DOM-
element and $elem the elem wrapped in a jQuery object. This would
avoid the different syntax in each area, like "curElem", which still
doesn't make clear whether it is a DOM or jQuery reference. (though
the setOffset method is so small that it's not a problem)
The same '$name' standard should apply to UI widgets. It makes object
pointers crystal clear. It also creates a clear relationship between
the DOM and jQuery objects, eg: $(menu) = $menu
Just my 2-cents.
We don't use Hungarian notation anywhere else, so this doesn't really make sense.
Kevin Dalman
> sense.
different from the conventions for events and methods names,
indentation, commenting, etc. It has become the defacto standard
adopted by most jQuery developers, including many of jQuery's own
developers. It facilitates easier reading and sharing of code, which
is why I suggested it. I've reviewed enough jQuery files to be annoyed
that vars like "elem" are often used for *both* DOM and jQuery
objects.
Many people *you know* specifically recommend it - for example:
"...here is a jQuery-specific naming convention that I highly
recommend. Use an initial $ on every variable that contains a jQuery
object:"
var $foo = $('#foo'), foo = foo[0];
$foo.show(); // jQuery
var id = foo.id; // DOM
"It's always a good idea to cache a jQuery object instead of repeating
the query or $() call over and over again, and the $ prefix is a good
visible reminder that it's a jQuery object."
- Mike Geary
Most jQuery books recommend it and use it in all their examples. For
example, from the jQuery Cookbook...
$element = $(this);
$element.addClass("clicked");
You'll also find it on the Learning jQuery website and other places,
so even jQuery newbies are taught to use it to make their code more
readable.
Plus, the convention is similar to other changes to the core...
"As of jQuery Revision 1371, the DOM Expando name has changed from
'events' to '$events'."
So I posted this suggestion not as a personal preference, but because
it is a widely used and published convention. Therefore it seemed
reasonable to *consider* whether core files and widgets should follow
the convention used by jQuery's own developers!
I'm not pushing the issue - just wanted to clarify.
Cheers
On Nov 16, 11:49 am, Scott González <scott.gonza...@gmail.com> wrote: