New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panel: Opening from anchor tag within listview does not remove active button class #5961

Closed
tschettler opened this Issue May 3, 2013 · 2 comments

Comments

Projects
None yet
2 participants
@tschettler

tschettler commented May 3, 2013

Opening a panel using an anchor tag within a list view does not remove the ui-btn-active class from the li containing the anchor.

Test Page: http://jsbin.com/idabam/2

Actual outcome: The "Open Panel" list item retains an active state.

Platforms/browsers tested:

  • iPhone 4 (Mobile Safari)
  • Android

jQuery Mobile 1.3.1
jQuery 1.9.1

This should work similar to how the popup widget handles removing the active class. Adding a delay before the active class is removed would also be a helpful UI cue.

Would a change similar to the following work?

_bindLinkListeners: function() {
    var self = this;

    self._page.on( "click.panel" , "a", function( e ) {
        if ( this.href.split( "#" )[ 1 ] === self._panelID && self._panelID !== undefined ) {
            e.preventDefault();
            var $link = $( this );
            if ( ! $link.hasClass( "ui-link" ) ) {
                $link.addClass( $.mobile.activeBtnClass );
                self.element.one( "panelopen panelclose", function() {
                    //remove after delay
                    setTimeout( function() {
                        $link.closest('li.' + $.mobile.activeBtnClass).andSelf().removeClass( $.mobile.activeBtnClass );
                    }, 300);
                });
            }
            self.toggle();
            return false;
        }
    });
},
@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot May 3, 2013

Member

@tschettler

Thanks for reporting the issue.

For 1.4 we are working on reducing the markup we generate for buttons, including the ones in listviews. As a result the anchor element in the list item will get the button classes, including the active state class. This resolves this issue.
It also means we can remove the code that was added (cb54f03) to fix the same issue we had with popups (#4611).

The ui-btn-active class is removed when the process of opening the panel is finished, so there is no need to use a timeout here.
The 300ms timeout we use in the popup widget was not part of the fix for #4611, but was added earlier (e612aa8). I am not completely sure why we choose to use a timeout.
@gabrielschulhof can't we remove the active state class on popupafteropen?

Member

jaspermdegroot commented May 3, 2013

@tschettler

Thanks for reporting the issue.

For 1.4 we are working on reducing the markup we generate for buttons, including the ones in listviews. As a result the anchor element in the list item will get the button classes, including the active state class. This resolves this issue.
It also means we can remove the code that was added (cb54f03) to fix the same issue we had with popups (#4611).

The ui-btn-active class is removed when the process of opening the panel is finished, so there is no need to use a timeout here.
The 300ms timeout we use in the popup widget was not part of the fix for #4611, but was added earlier (e612aa8). I am not completely sure why we choose to use a timeout.
@gabrielschulhof can't we remove the active state class on popupafteropen?

@ghost ghost assigned jaspermdegroot May 3, 2013

@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot May 3, 2013

Member

I just realized that we should fix this for 1.3.2. I will apply the same fix as we did for popups.

Member

jaspermdegroot commented May 3, 2013

I just realized that we should fix this for 1.3.2. I will apply the same fix as we did for popups.

jaspermdegroot added a commit that referenced this issue May 3, 2013

jaspermdegroot added a commit that referenced this issue May 3, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment