Menu: Refactored next/previousPage logic and activate-scrolling, improved much!

diff --git a/tests/visual/menu/menu.html b/tests/visual/menu/menu.html
index 6388b10537369091c73a22df38d1ff4ae8bce4f2..1247fd7ea611b881ca0ee936a4535dbaa140224d 100644 (file)
--- a/tests/visual/menu/menu.html
+++ b/tests/visual/menu/menu.html
@@ -8,7 +8,6 @@
        <script type="text/javascript" src="../../../ui/jquery.ui.core.js"></script>
        <script type="text/javascript" src="../../../ui/jquery.ui.widget.js"></script>
        <script type="text/javascript" src="../../../ui/jquery.ui.menu.js"></script>
-       <script type="text/javascript" src="http://jqueryui.com/themeroller/themeswitchertool/"></script>
        <script type="text/javascript">
        $(function() {
                $.fn.themeswitcher && $('<div/>').css({

diff --git a/ui/jquery.ui.menu.js b/ui/jquery.ui.menu.js
index 94df804f6f4ad55aa183aa78d56ccafc1d9ff8c8..58127566d567fd239ba81bcd1dcb697d88a2ddb8 100644 (file)
--- a/ui/jquery.ui.menu.js
+++ b/ui/jquery.ui.menu.js
@@ -122,13 +122,16 @@ $.widget("ui.menu", {
        activate: function( event, item ) {
                this.deactivate();
                if ( this._hasScroll() ) {
-                       var offset = item.offset().top - this.element.offset().top,
+                       var borderTop = parseFloat( $.curCSS( this.element[0], "borderTopWidth", true) ) || 0,
+                               paddingtop = parseFloat( $.curCSS( this.element[0], "paddingTop", true) ) || 0,
+                               offset = item.offset().top - this.element.offset().top - borderTop - paddingtop,
                                scroll = this.element.attr( "scrollTop" ),
-                               elementHeight = this.element.height();
-                       if (offset < 0) {
+                               elementHeight = this.element.height(),
+                               itemHeight = item.height();
+                       if ( offset < 0 ) {
                                this.element.attr( "scrollTop", scroll + offset );
-                       } else if (offset > elementHeight) {
-                               this.element.attr( "scrollTop", scroll + offset - elementHeight + item.height() );
+                       } else if ( offset + itemHeight > elementHeight ) {
+                               this.element.attr( "scrollTop", scroll + offset - elementHeight + itemHeight );
                        }
                }
                this.active = item.first()
@@ -187,19 +190,16 @@ $.widget("ui.menu", {
                        }
                        var base = this.active.offset().top,
                                height = this.element.height(),
-                               // TODO replace children with nextAll
-                               // TODO replace filter with each, break once close > 0 and use that item as the result 
-                               result = this.element.children( ".ui-menu-item" ).filter( function() {
-                                       var close = $( this ).offset().top - base - height + $( this ).height();
-                                       // TODO replace with check close > 0
-                                       return close < 10 && close > -10;
-                               });
-
-                       // TODO try to catch this earlier when scrollTop indicates the last page anyway
-                       if ( !result.length ) {
-                               result = this.element.children( ".ui-menu-item" ).last();
-                       }
-                       this.activate( event, result );
+                               result;
+                       this.active.nextAll( ".ui-menu-item" ).each( function() {
+                               var close = $( this ).offset().top - base - height;
+                               if (close >= 0) {
+                                       result = $( this );
+                                       return false;
+                               }
+                       });
+
+                       this.activate( event, result || this.element.children( ".ui-menu-item" ).last() );
                } else {
                        this.activate( event, this.element.children( ".ui-menu-item" )
                                // TODO use .first()/.last()
@@ -216,18 +216,17 @@ $.widget("ui.menu", {
                        }
 
                        var base = this.active.offset().top,
-                               height = this.element.height();
-                               result = this.element.children( ".ui-menu-item" ).filter( function() {
-                                       var close = $(this).offset().top - base + height - $(this).height();
-                                       // TODO improve approximation
-                                       return close < 10 && close > -10;
-                               });
-
-                       // TODO try to catch this earlier when scrollTop indicates the last page anyway
-                       if (!result.length) {
-                               result = this.element.children( ".ui-menu-item" ).first();
-                       }
-                       this.activate( event, result );
+                               height = this.element.height(),
+                               result;
+                       this.active.prevAll( ".ui-menu-item" ).each( function() {
+                               var close = $(this).offset().top - base + height;
+                               if (close <= 0) {
+                                       result = $( this );
+                                       return false;
+                               }
+                       });
+
+                       this.activate( event, result || this.element.children( ".ui-menu-item" ).first() );
                } else {
                        this.activate( event, this.element.children( ".ui-menu-item" )
                                // TODO use .first()/.last()