Usability bug: Upcoming/Popular tabs should remain in place

Well, you wrote the thing. It would take me days to decipher what you did by myself. So let's take a look at the code and see if there's a fix.

So that top bar is generated by a call to a function called drigg_ui_type_menu()

Here's the code for that function from the drigg_ui module file.

My suspicion is that somewhere in here, you're telling the function not to print the active link but only the inactive ones (if you're on the popular page, only print upcoming and archived).

So if we can find the bit of code that's "killing" the active link, maybe that will work?

/**
 * Returns the top menu ("popular", "upcoming", "archived")
 * This function ALSO preservs all of the links, whenever possible.
 *
 * @return
 *   The top menu
*/
function drigg_ui_type_menu(){


  $output='';
  $selected_string=' class="active" ';

  $p0=arg(0);
  $p1=arg(1);
  $p2=arg(2);

  // This will make sure that even the home page will work OK
  if ( (arg(0)=='node' || arg(0)== variable_get('drigg_home_name', 'drigg_home') ) && arg(1) == '' ){
    $p0='popular';
    $p1='newest';
  }

Ok. So this first part sets some defaults for the two filter sets for the home page. Cool. Doesn't look like this is the code we need to change.

  // This will make sure that even /Section works fine
  if (drigg_is_section_valid( $p0 ) && $p1 == '' ){
    $p0='popular';
    $p1='newest';
    $p2=arg(0);
  }

  // This avoids doing anything if the URL is incorrect
  if($p0 != 'popular' && $p0 != 'upcoming' && $p0 != 'archived'){
    return '';
  }

This doesn't look like it either.

  // Hide the "archived" menu entry, UNLESS you _ARE_ in "archived"
  if(variable_get('drigg_hide_archived_menu', FALSE) && $p0 != 'archived'){
    $items=array('popular','upcoming');
  } else {
    $items=array('popular','upcoming','archived');
  }

Nor this.

  // For each item, prints it, but with a twist...
  // At this point, things are "normalised". So, no matter what,
  // the format is for example popular/top24h/Community or
  // popular/top24h
  foreach( $items as $item ){
    $selected = ''; 

    $pp1=$p1;

    // Mark it as selected if it's the same as the current one
    if($item == $p0 ){
      $selected = $selected_string; 
    }

*** Could this bit above be the thing that's stopping the function from outputting the active link? ***

Here's the rest of the function.

    // Last minute change: you can't jump from 'popular/top24h' to
    // 'upcoming/24h'!!! So, if things are not right, fix them on the
    // spot. From now on, $pp1 needs to be considered, rather than
    // $p1
    //
    if($item == 'upcoming' || $item == 'archived'){
      if($p1 != 'oldest' && $p1 != 'mostpopular' && $p1 != 'leastpopular'){
        $pp1 = 'newest';
      }
    }
    if($item == 'popular'){
      if($p1 != 'top7days' && $p1 != 'top24h' && $p1 != 'top30days' && $p1 != 'top365days'){
        $pp1 = 'newest';
      }
    }

    // If the link is going to be /popular/newest, then change it
    // last minute to just /
    if($item == 'popular' && $pp1 == 'newest' && $p2 == ''){
      $link = drigg_ui_home_url();
    // Otherwise, business as normal
    } else {
      $link="$item";
      if($pp1 != ''){ $link .= "/$pp1"; }
      if($p2 != '') { $link .= "/$p2"; }
      #$link="/$item" .  $pp1 != '' ? "/$pp1/$p2" : ''; 
    }

    // NOW that link is set, I will add the Drupal's root to it
    // in case there is one
    //
    #$bp=base_path();
    #if($bp != '/'){
    #  $link=substr($bp,0,-1).$link;
    #}

    $output.="<li $selected><a ".$selected.' href="'.url($link).'">'.t($item) .'</a></li>';
      
  }
  return "<ul class=\"drigg-viewtype\">$output</ul>";
   
}

Did I find the right spot in the function? If so, what do we do to it now?

Ahhh. Ok. I see where the break is between what we're saying.

So here's how it works right now.

On the home page in the view type bar you have 3 elements (if archived view is enabled, 2 if not)

Popular Upcoming Archived

Picture those as tabs, with the popular one being the active tab.

Now, currently, the active tab isn't really a tab.....it's a static title (an H1 in the Drigg theme) while the other two are actual links.

So you've got: H1(Popular) link(Upcoming) link(Archived)

When you click one of the links (Upcoming, for example), the link you clicked on shows up as the page title tab and you get:

H1(Upcoming) Link(Popular) llink(Archived)

So that's what's happening now........

The problem isn't just using CSS to get the H1 tab to stay in position.

The problem is that we hare

H1 link link

When we should have

link link link

This way the H1, the page title can be used someplace else (like on Digg, where you see it above the content) and we can have actual fixed tabs for the view type controls.

Now I know this is possible to do because the controls for the time filter (drigg_order) are: link link link link link

So that means its possible to set up the Drigg views controls that way, too.

What I tried to do in my post above was outline where I thought the code was "holding back" the active link so that it wasn't duplicated by the h1 tab. Right now, when I click the upcoming link, the "upcoming" link disappears. That's gotta be by design so that it won't duplicate the h1 element. What we need to do is stop it from disappearing so we can theme it as a tab.

So I'm not proposing a CSS fix to the theme. I'm proposing that we remove the Page title/H1 from the view controls and replace it with an actual working link, just like with the time controls and the category controls.

Cool.

The Problem:

When you click on one of the order links ("top24h" for example) all of those links are there on the next page.

But when you click on one of the view type links, the one you just clicked on isn't there anymore. Something happened to it. I'm guessing this is happening somewhere in the function itself.

I was just summing it up again for you. same issue. Feel free to ignore the last one. It was an attempt at a concise version of my previous message.

Great work on this one. I now see what was happening. ha! It was just a "display:none" in the CSS. Weird. Thanks so much for making the change. Much more useable now.