The "selected" feature wasn't working so I tracked down through the code and found the issue.

This used to be working because it looked for the active class on the <a>. However, in the latest commit to the fork JS it switched to look on the <li>. In theory either one should be fine, but since "active" is usually added by the l() function for Drupal it probably makes more sense to keep it looking at the <a>.

This is literally a two character patch.

The relevant diff to see the change is here: http://drupalcode.org/project/tinynav.git/blobdiff/5e8016dafd6720b36bc90...

CommentFileSizeAuthor
#1 0001-Issue-2212963-Change-looking-from-li-to-a-for-select.patch1.16 KBsokrplare
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sokrplare’s picture

sokrplare CreditAttribution: sokrplare commented
Status: Active » Needs review
FileSize
0001-Issue-2212963-Change-looking-from-li-to-a-for-select.patch1.16 KB
btopro’s picture

btopro CreditAttribution: btopro commented

can we get a second or third in terms of reviewing this patch? I see exactly what you mean, just want some confirmation from a wider audience. Makes sense to me

dflitner’s picture

dflitner CreditAttribution: dflitner commented

Patch in #1 worked for me. I'm using 7.x-1.3, not the -dev version.

Background:
I have two menus I want to use tinynav for.
I changed my active class to "active" from "active-trail" because that's what the module-created menu uses (QuickTabs). This made the second menu which is a regular Drupal menu not get properly selected in tinynav mode because the li has an "active-trail" class and the a has only an "active" class. Applied this patch and now the current page is correctly selected in the tinynav menu.

btopro’s picture

btopro CreditAttribution: btopro at Pennsylvania State University commented
Status: Needs review » Fixed

needed to rebase but applied to dev. thanks for testing, accepted.

  • btopro committed ce30b82 on 7.x-1.x authored by covenantd 
    Issue #2212963 by covenantd: Selected looking at <li> instead of <a> for...
tiikeri’s picture

tiikeri CreditAttribution: tiikeri commented

I was writing just about the same problem, but the patch here fixed it. Thanks

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.