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

Arrow on formatted listviews no longer clickable #1392

Closed
matellis opened this Issue Apr 7, 2011 · 17 comments

Comments

Projects
None yet
9 participants
@matellis

matellis commented Apr 7, 2011

This looks like a regression. In formatted listviews the right arrow icon no longer does anything when you click on it.

Steps to reproduce:

  1. See http://jquerymobile.com/test/#docs/lists/lists-formatting.html
  2. Attempt to click on the right arrow does nothing.

The structure inside a list item element is now:

  • In A3 and before the arrow would be inside the a href and would be clickable.

    One can hack the situation and insert a second link to the same place with no text and then override styles in the split view.

    @telbert

    This comment has been minimized.

    Show comment
    Hide comment
    @telbert

    telbert Apr 20, 2011

    Hi,

    short note:

    Due to the right padding of 75px (.ui-li .ui-btn-inner a.ui-link-inherit, .ui-li-static.ui-li) in the tag within list items, it's not possible to trigger the navigation on the data-icon.
    (extracted from closed issue #1474 )

    Regards
    telbert

    telbert commented Apr 20, 2011

    Hi,

    short note:

    Due to the right padding of 75px (.ui-li .ui-btn-inner a.ui-link-inherit, .ui-li-static.ui-li) in the tag within list items, it's not possible to trigger the navigation on the data-icon.
    (extracted from closed issue #1474 )

    Regards
    telbert

    @telbert

    This comment has been minimized.

    Show comment
    Hide comment
    @telbert

    telbert May 3, 2011

    It seems, that the padding is not the issue. Only the icon does not work. Clicking on the padding area will also navigate to the relevant link.

    Strange thing is: In A3, the icon itself is not in the <a> -Tag, too, but it works.

    A3:

    <li class="ui-btn ui-btn-icon-right ui-li ui-btn-up-c" role="option" tabindex="0" data-theme="c">
     <div class="ui-btn-inner">
      <div class="ui-btn-text">
       <a class="ui-link-inherit" href="index.html">Acura</a>
      </div>
      <span class="ui-icon ui-icon-arrow-r"></span>
     </div>
    </li>
    

    A4:

    <li class="ui-btn ui-btn-icon-right ui-li ui-btn-hover-c" data-theme="c">
     <div class="ui-btn-inner ui-li">
      <div class="ui-btn-text">
       <a class="ui-link-inherit" href="index.html">Acura</a>
      </div>
      <span class="ui-icon ui-icon-arrow-r"></span>
     </div>
    </li>
    

    Any ideas?

    telbert commented May 3, 2011

    It seems, that the padding is not the issue. Only the icon does not work. Clicking on the padding area will also navigate to the relevant link.

    Strange thing is: In A3, the icon itself is not in the <a> -Tag, too, but it works.

    A3:

    <li class="ui-btn ui-btn-icon-right ui-li ui-btn-up-c" role="option" tabindex="0" data-theme="c">
     <div class="ui-btn-inner">
      <div class="ui-btn-text">
       <a class="ui-link-inherit" href="index.html">Acura</a>
      </div>
      <span class="ui-icon ui-icon-arrow-r"></span>
     </div>
    </li>
    

    A4:

    <li class="ui-btn ui-btn-icon-right ui-li ui-btn-hover-c" data-theme="c">
     <div class="ui-btn-inner ui-li">
      <div class="ui-btn-text">
       <a class="ui-link-inherit" href="index.html">Acura</a>
      </div>
      <span class="ui-icon ui-icon-arrow-r"></span>
     </div>
    </li>
    

    Any ideas?

    @siromega

    This comment has been minimized.

    Show comment
    Hide comment
    @siromega

    siromega May 11, 2011

    4 and 4.1 appeared to have cleaned up the implementation for listviews, but the elimination of some code in the listview widget _create() function appears to be the culprit. In alpha 3, the code would handle any click on a LI or child element and then trigger the first A element it found.

    The code from alpha 3:

        // tapping the whole LI triggers click on the first link
        $list.delegate( "li", "click", function(event) {
          if ( !$( event.target ).closest( "a" ).length ) {
            $( this ).find( "a" ).first().trigger( "click" );
            return false;
          }
        }); 
    

    while that code is removed in alpha 4 because the link takes up all the space (whereas it used to just take up only the room for the needed text as an inline element), it misses the clicks on the icon. Logging the click event in Firebug showed that while there was an event on the UL (the delegate above), in alpha 4 there is none, and the currentTarget value on the icon click event is null (indicating there is no event handler to catch that event).

    I guess the solutions are - put the code above back in to handle the icon click event bubbling up, or figure out a way to put the icon inside the link. This was the workaround I came up with (probably should be formalized and cleaned up), I added a check to make sure the source of the click event is the ui-icon class. Around line 4600....

        $list.delegate( "li", "click", function(event) {
          if ( !$( event.target ).closest( "a" ).length && $( event.target ).hasClass( "ui-icon" ) ) {
            $( this ).find( "a" ).first().trigger( "click" );
            return false;
          }
        }); 
    

    siromega commented May 11, 2011

    4 and 4.1 appeared to have cleaned up the implementation for listviews, but the elimination of some code in the listview widget _create() function appears to be the culprit. In alpha 3, the code would handle any click on a LI or child element and then trigger the first A element it found.

    The code from alpha 3:

        // tapping the whole LI triggers click on the first link
        $list.delegate( "li", "click", function(event) {
          if ( !$( event.target ).closest( "a" ).length ) {
            $( this ).find( "a" ).first().trigger( "click" );
            return false;
          }
        }); 
    

    while that code is removed in alpha 4 because the link takes up all the space (whereas it used to just take up only the room for the needed text as an inline element), it misses the clicks on the icon. Logging the click event in Firebug showed that while there was an event on the UL (the delegate above), in alpha 4 there is none, and the currentTarget value on the icon click event is null (indicating there is no event handler to catch that event).

    I guess the solutions are - put the code above back in to handle the icon click event bubbling up, or figure out a way to put the icon inside the link. This was the workaround I came up with (probably should be formalized and cleaned up), I added a check to make sure the source of the click event is the ui-icon class. Around line 4600....

        $list.delegate( "li", "click", function(event) {
          if ( !$( event.target ).closest( "a" ).length && $( event.target ).hasClass( "ui-icon" ) ) {
            $( this ).find( "a" ).first().trigger( "click" );
            return false;
          }
        }); 
    

    @ghost ghost assigned johnbender Jul 27, 2011

    @johnyanarella

    This comment has been minimized.

    Show comment
    Hide comment
    @johnyanarella

    johnyanarella Jul 30, 2011

    Any update on this? Still seeing this issue even now as the project heads into Beta 2.

    Currently patching the issue locally with siromega's suggested addition to the mobile.listview plugin's _create method.

    johnyanarella commented Jul 30, 2011

    Any update on this? Still seeing this issue even now as the project heads into Beta 2.

    Currently patching the issue locally with siromega's suggested addition to the mobile.listview plugin's _create method.

    @johnyanarella

    This comment has been minimized.

    Show comment
    Hide comment
    @johnyanarella

    johnyanarella Jul 31, 2011

    As it turns out, that workaround isn't viable for rel=external anchors.

    Instead, I'm currently using the following (unoptimized) addition:

    a.first().appendTo( item );
    a.first().contents().appendTo( item.find( ".ui-btn-text" ) );
    item.children( ".ui-btn-inner" ).appendTo( a.first() );
    

    to the refresh() method, just after buttonMarkup() and the 'ui-link-inherit' class are applied to the item.

    This reorders the HTML tags so that the anchor contains the inner button content, matching what's done with anchors in jQuery Mobile navbars, etc.

    For example, this:

    <li data-theme="c" class="ui-btn ui-btn-icon-right ui-li ui-corner-top ui-btn-down-c ui-btn-up-c">
        <div class="ui-btn-inner">
            <div class="ui-btn-text">
                <a href="guides.html" rel="external" class="ui-link-inherit">Guides</a>
            </div>
            <span class="ui-icon ui-icon-arrow-r ui-icon-shadow"></span>
        </div>
    </li>
    

    becomes this:

    <li data-theme="c" class="ui-btn ui-btn-icon-right ui-li ui-corner-top ui-btn-down-c ui-btn-up-c">
        <a href="guides.html" rel="external" class="ui-link-inherit">
            <div class="ui-btn-inner">
                <div class="ui-btn-text">Guides</div>
                <span class="ui-icon ui-icon-arrow-r ui-icon-shadow"></span>
            </div>
        </a>
    </li>
    

    johnyanarella commented Jul 31, 2011

    As it turns out, that workaround isn't viable for rel=external anchors.

    Instead, I'm currently using the following (unoptimized) addition:

    a.first().appendTo( item );
    a.first().contents().appendTo( item.find( ".ui-btn-text" ) );
    item.children( ".ui-btn-inner" ).appendTo( a.first() );
    

    to the refresh() method, just after buttonMarkup() and the 'ui-link-inherit' class are applied to the item.

    This reorders the HTML tags so that the anchor contains the inner button content, matching what's done with anchors in jQuery Mobile navbars, etc.

    For example, this:

    <li data-theme="c" class="ui-btn ui-btn-icon-right ui-li ui-corner-top ui-btn-down-c ui-btn-up-c">
        <div class="ui-btn-inner">
            <div class="ui-btn-text">
                <a href="guides.html" rel="external" class="ui-link-inherit">Guides</a>
            </div>
            <span class="ui-icon ui-icon-arrow-r ui-icon-shadow"></span>
        </div>
    </li>
    

    becomes this:

    <li data-theme="c" class="ui-btn ui-btn-icon-right ui-li ui-corner-top ui-btn-down-c ui-btn-up-c">
        <a href="guides.html" rel="external" class="ui-link-inherit">
            <div class="ui-btn-inner">
                <div class="ui-btn-text">Guides</div>
                <span class="ui-icon ui-icon-arrow-r ui-icon-shadow"></span>
            </div>
        </a>
    </li>
    
    @webmat

    This comment has been minimized.

    Show comment
    Hide comment
    @webmat

    webmat Sep 12, 2011

    Still seeing this in Beta 3 (try clicking the arrow icons here: http://jquerymobile.com/demos/1.0b3/ ).

    Any update on when this will be fixed?

    webmat commented Sep 12, 2011

    Still seeing this in Beta 3 (try clicking the arrow icons here: http://jquerymobile.com/demos/1.0b3/ ).

    Any update on when this will be fixed?

    @hugeupside

    This comment has been minimized.

    Show comment
    Hide comment
    @hugeupside

    hugeupside Sep 12, 2011

    Short note in support of prioritizing resolution. In beta testing a small web app with ~ 50 users, 2 independently identified this issue and submitted bugs.

    hugeupside commented Sep 12, 2011

    Short note in support of prioritizing resolution. In beta testing a small web app with ~ 50 users, 2 independently identified this issue and submitted bugs.

    @toddparker

    This comment has been minimized.

    Show comment
    Hide comment
    @toddparker

    toddparker Sep 22, 2011

    Contributor

    Hi all -

    I just tested latest and was able to click the right arrows in a few devices. Marking this as closed, but will re-open if others report this is still happening.

    Contributor

    toddparker commented Sep 22, 2011

    Hi all -

    I just tested latest and was able to click the right arrows in a few devices. Marking this as closed, but will re-open if others report this is still happening.

    @toddparker toddparker closed this Sep 22, 2011

    @johnyanarella

    This comment has been minimized.

    Show comment
    Hide comment
    @johnyanarella

    johnyanarella Sep 22, 2011

    Is that the same latest deployed here (or is it newer)?

    http://code.jquery.com/mobile/latest/demos/

    No luck clicking the arrows in Safari, Chrome or Firefox.

    johnyanarella commented Sep 22, 2011

    Is that the same latest deployed here (or is it newer)?

    http://code.jquery.com/mobile/latest/demos/

    No luck clicking the arrows in Safari, Chrome or Firefox.

    @toddparker

    This comment has been minimized.

    Show comment
    Hide comment
    @toddparker

    toddparker Sep 22, 2011

    Contributor

    @johnyanarella - I tested this on a bunch of phones, but now I'm seeing that on desktop browsers, the mouse events are the issue, not touch. That could be that touch events have larger tolerances so even thouhg I was trying to just tap the arrow, the row click was triggered. Tested here:
    http://jquerymobile.com/test/

    Re-opening.

    Contributor

    toddparker commented Sep 22, 2011

    @johnyanarella - I tested this on a bunch of phones, but now I'm seeing that on desktop browsers, the mouse events are the issue, not touch. That could be that touch events have larger tolerances so even thouhg I was trying to just tap the arrow, the row click was triggered. Tested here:
    http://jquerymobile.com/test/

    Re-opening.

    @toddparker toddparker reopened this Sep 22, 2011

    Wilto added a commit to Wilto/jquery-mobile that referenced this issue Sep 22, 2011

    Fixes #1392 — Positions .ui-icon on lower z-index than .ui-btn-text, …
    …ensuring the click will register on the latter.

    Wilto added a commit to Wilto/jquery-mobile that referenced this issue Sep 27, 2011

    Wilto pushed a commit to Wilto/jquery-mobile that referenced this issue Sep 28, 2011

    Merge pull request #2556 from Wilto/master
    Assorted CSS adjustments. Fixes #1392, Fixes #2513. Thanks Wilto!
    @blq

    This comment has been minimized.

    Show comment
    Hide comment
    @blq

    blq Oct 11, 2011

    I think this should be reopened.
    Clicking the icon doesn't work on for example: Samsung Galaxy Tab, HTC Legend, Firefox 7, Safari 5.1 or Chrome 14.

    (Using the "latest" branch, as of today)

    blq commented Oct 11, 2011

    I think this should be reopened.
    Clicking the icon doesn't work on for example: Samsung Galaxy Tab, HTC Legend, Firefox 7, Safari 5.1 or Chrome 14.

    (Using the "latest" branch, as of today)

    @blq

    This comment has been minimized.

    Show comment
    Hide comment
    @blq

    blq Oct 11, 2011

    update: using the CSS from RC1 it works though.

    blq commented Oct 11, 2011

    update: using the CSS from RC1 it works though.

    @toddparker

    This comment has been minimized.

    Show comment
    Hide comment
    @toddparker

    toddparker Oct 11, 2011

    Contributor

    So what were you testing on that earlier message? Latest? Just want to know if this was working for RC1 but broke after.

    Contributor

    toddparker commented Oct 11, 2011

    So what were you testing on that earlier message? Latest? Just want to know if this was working for RC1 but broke after.

    @blq

    This comment has been minimized.

    Show comment
    Hide comment
    @blq

    blq Oct 11, 2011

    When using http://code.jquery.com/mobile/latest/jquery.mobile.css as of today, this very minute :), it does Not work on the mentioned platforms. If I use the CSS from RC1 it does work. So, yes seems something broke recently.

    blq commented Oct 11, 2011

    When using http://code.jquery.com/mobile/latest/jquery.mobile.css as of today, this very minute :), it does Not work on the mentioned platforms. If I use the CSS from RC1 it does work. So, yes seems something broke recently.

    @toddparker

    This comment has been minimized.

    Show comment
    Hide comment
    @toddparker

    toddparker Oct 11, 2011

    Contributor

    Tganks. Can you test those devices on jquerymobile.com/test to make sure? We may have tweaked JS too. The URL is the fastest way to preview the latest.

    When using http://code.jquery.com/mobile/latest/jquery.mobile.css as of today, this very minute :), it does Not work on the mentioned platforms. If I use the CSS from RC1 it does work.

    Reply to this email directly or view it on GitHub:
    #1392 (comment)

    Contributor

    toddparker commented Oct 11, 2011

    Tganks. Can you test those devices on jquerymobile.com/test to make sure? We may have tweaked JS too. The URL is the fastest way to preview the latest.

    When using http://code.jquery.com/mobile/latest/jquery.mobile.css as of today, this very minute :), it does Not work on the mentioned platforms. If I use the CSS from RC1 it does work.

    Reply to this email directly or view it on GitHub:
    #1392 (comment)

    @blq

    This comment has been minimized.

    Show comment
    Hide comment
    @blq

    blq Oct 11, 2011

    Ok, done. Same result I'm afraid. I.e doesn't work on any of the devices above (works on iPad 2 & iPod).

    blq commented Oct 11, 2011

    Ok, done. Same result I'm afraid. I.e doesn't work on any of the devices above (works on iPad 2 & iPod).

    @toddparker

    This comment has been minimized.

    Show comment
    Hide comment
    @toddparker

    toddparker Oct 11, 2011

    Contributor

    Thanks so much for testing, we'll take another look.

    Ok, done. Same result I'm afraid. I.e doesn't work on any of the devices above (works on iPad & iPod).

    Reply to this email directly or view it on GitHub:
    #1392 (comment)

    Contributor

    toddparker commented Oct 11, 2011

    Thanks so much for testing, we'll take another look.

    Ok, done. Same result I'm afraid. I.e doesn't work on any of the devices above (works on iPad & iPod).

    Reply to this email directly or view it on GitHub:
    #1392 (comment)

    @toddparker toddparker reopened this Oct 11, 2011

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