No active tab in list view

[achartier]

I am displaying a list and set the tab field to an element called category (field element). I do not have an All tab. When the list first opens from the menu no tabs are active. Please see image one.

In components/com_fabrik/models/list.php the tabs entries are built. Function loadTabs (11984) at line number 12042 sets the row class for the active tab. It does this by comparing the current URI to the href built for the tab. However, this only works when the tabField=xxx is in the current URI. The tabField is not in the URI when the list view is called from a standard Fabrik list view menu. Therefore, with no tabField defined in the URI no tab is set active.

The existing code at line 12042 looks like this:

$row->class = ($thisUri == $row->href) ? 'active' : '';

To solve this problem we can change the code to read:

$row->class = ($thisUri == $row->href || (strpos($thisUri, $tabsField) === false && $i == 0)) ? 'active' : '';

With this change the tabs are set properly as shown in image_two.

 

[cheesegrits]

I've committed that to github:

https://github.com/Fabrik/fabrik/commit/d1231d53417bdd5e5b9180e1233da7df99e21bb0

Thanks. I don't ever use tabbed list view, and it was a feature a user contributed, so I'm not at all familiar with the code.

-- hugh

 

[Sophist]

It was something I contributed several years ago (which means that my memory about the algorithm is not great).

As far as I can remember, these are not real tabs (i.e. showing and hiding parts of the page using JS), but instead pseudo tabs which reload the page with different filters.

I agree that the existing (well ... now previous) code was not ideal. It only made a tab active if the URL matched exactly - which means that (as you have found) you can easily get a situation where the current url is not an exact match to any of the tab URLs, and then you don't get any tab active. Unfortunately the original post does not give details of the page URL and the tab URLs, so I cannot make any judgement (yet) about whether this is the best approach.

The difficulty is in judging which is the best tab to make active when the URL does not match exactly. This code attempts to make the first tab active as a default, but I fear that this is not the correct approach, for two reasons:

1. You may end up with two tabs shown as active. If I have read this code correctly, I suspect that when you click e.g. the second tab, you might end up with both the first and second tabs active;
2. The first tab might not be the best tab to be made active.

For the first of these points, if I were coding this fix I would keep the code as it was inside the loop, but add a boolean to track whether we have set a tab active. Immediately after the loop I would check to see if we have activated a tab and if not then activate the first one. This would be something like this (untested):

$tabActivated = false;
foreach ($tabs as $i => $tabArray)
{
    $row = new stdClass;

...

    $row->js = false;
    // The current test for active tab is simplistic if initial list display is using a URL which does not exactly match a tab filter.
    // It may be possible to be more intelligent e.g. by testing just the filter parameters
    $activateTab = $thisUri == $row->href;
    $tabActivated = $tabActivated || $activateTab;
    $row->class = $activateTab ? 'active' : '';
    $this->tabs[] = $row;
}

if (!$tabActivated)
{
    $this->tabs[0]->class = 'active';
}

 

[achartier]

The above world work. In my code I test whether the tabs field is in the URI, if it is not then I set the first tab as active.

I have also noticed on occasion that the first tab lists all items irrespective of the tab field. It doesn't happen all the time and I haven't looked at it in depth. Has this ever happened with any other implementation?

 

[achartier]

So it appears that the first time a user opens the list, all items are showing on the first tab as if it was an All tab. There is no initial filter set in the menu (which is why we need to set the active tab). Setting a prefilter doesn't work because then the entire list is restricted to that filter.

I attempted to insert a filter into the application->input when the active tab is set but the list row data has already gone through the filter process and all the rows are already included in the list.

I can call the list from an external url so I can add the initial tab filter but that seems like the wrong answer to me.

Any thoughts?

 

[achartier]

Sophist, the code I provided would not set 2 active tabs. It verifies whether there is a tabField filter set in the URL, which would be the case for any click on a tab. In the case of a menu link there is no tabField in the URL so my code sets the first tab as the default.

Having said that, I think the comparison of url's is probably not the best approach. The easier approach would be to simply look for the tabField in the URL and make whatever it is set to as the active tab. If no tabField is in the URL then make tab zero the active tab. I think comparing a long URL is fraught with danger.

I am happy to write it up and PR it if you agree.

Then, to solve the initial display of all rows....

 

[Sophist]

Yes - comparing the full URL is fraught with danger because it could have other parameters other than filters which mean it is effectively the same filters as a tab but not an exact match. So a more intelligent comparison would be better. And the URLs for the tabs should perhaps have non-filter parameters for the current URL added - so if they are included in the initial URL, then they are retained when you click the tabs.

But it does seem to me that with your code if you click a different tab, then it will match the URL for that tab exactly, so that tab will be set active. But it won't match the strpos for the first tab (i==0) so that will be set active also. But I cannot remember what the variable you are comparing with strpos contains, so I might be wrong.

I guess the right way to stop the display of all rows would be:

1. Check whether the URL matches one of the tabs, and if so just display with that tab active; but if not...
   
   
2. Redirect to the URL for the first tab i.e. so that it h

though this may not be needed if you replicate the non-filter parameters from the initial URL.

 

[achartier]

But it does seem to me that with your code if you click a different tab, then it will match the URL for that tab exactly, so that tab will be set active. But it won't match the strpos for the first tab (i==0) so that will be set active also. But I cannot remember what the variable you are comparing with strpos contains, so I might be wrong.

The code I submitted sets the active tab if there is a URL/URI match. If the URL does not have the tabField in it (which is what the tab URLs have) AND it is the first tab, we set the first tab active.

I agree that we should include other URL parameters in the tab url. This should not be so hard.

I like the idea of doing a redirect on the initial entry if no tabField is defined in the URL, that would solve the problem with the first tab showing all the records.

Should I code it up and PR it?

 

[cheesegrits]

Sure.

Just test it thoroughly first.

-- hugh

 

[achartier]

Will do. Probably won't get to it until Thursday though.

 

[cheesegrits]

That's OK, I probably won't get round to merging it for a year or so.

I owe Paul a merge session, so as soon as this is ready, I'll put aside an hour or two for just that ...

-- hugh

 

[achartier]

Paul & I spent some time reviewing the implementation and we are both happy with it. I have submitted a PR. Can you merge when you get a chance please?

Thanks.

 

[cheesegrits]

Done.

-- hugh

 

[achartier]

Thanks, I will monitor the forum to see if anyone reports any issues.