?
avatar brianteeman
brianteeman
30 Apr 2017

Currently we save the tab state so that if you are on the third tab and press save you are not redirected to the first tab.

However the tab state is persistent so if you save and close an article etc on the third tab then when you open a new article etc you are on the third tab.

To me this is a user experience error and the tab state should be reset/cleared when you go to a new article etc

??

avatar brianteeman brianteeman - open - 30 Apr 2017
avatar joomla-cms-bot joomla-cms-bot - change - 30 Apr 2017
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 30 Apr 2017
avatar Bakual
Bakual - comment - 30 Apr 2017

However the tab state is persistent so if you save and close an article etc on the third tab then when you open a new article etc you are on the third tab.

To be more specific, it is persistent only as long as the current session exists. After closing the browser the tab state is resetted/cleared with Joomla 3.7.0.

However i think that issue is a valid one.
Basically the tab state storing is only useful for the case where you click the "Save" (not "Save & Close") button in eg an article so you stay on the tab you had active before saving. Once closing the item, the state actually could be cleared.

avatar brianteeman
brianteeman - comment - 30 Apr 2017

In addition the persistent tab.state prevents linking to specific tabs. Even though you can build the url with an anchor ref to the correct tab the stored state will prevent it from working

avatar tonypartridge
tonypartridge - comment - 30 Apr 2017

I've been having this issue for a while and completely agree with you Brian the system needs a complete fix. At present a fix was implement into 3.7 so it was browser specific and not system specific.

Realistically the save / save & close states need to be take into consideration. So on save / apply it stores the tab state, on save & close or cancel the tab state is cleared.

But I don't know the JavaScript codebase or logic to know if this is possible or if we need to rewrite it with new logic.

avatar franz-wohlkoenig franz-wohlkoenig - change - 1 May 2017
Category Feature Request UI/UX
avatar ggppdk
ggppdk - comment - 1 May 2017

The index for selecting active at file:
media/system/js/tab_state.js

is the current URL after removing:

  • return=...
  • somename=NNNN (e.g. in our case id=47 or myvar=25 will be removed)

It seems wrong to me to leave all responsibilty for setting the active TAB into all-cases handling code

Current page (component view) knows better if the active TAB should be remember or not,
or at least it should be possible to skip activating a TAB automatically

I think we need a new data-option at an appropriate HTML tag or new JS method
to tell tab_state.js if the active tab should be set according to its index

Maybe @dgt41 would know best way to do this

avatar franz-wohlkoenig franz-wohlkoenig - change - 1 May 2017
Status New Discussion
avatar brianteeman
brianteeman - comment - 2 May 2017

@ggppdk i dont agree that we need a new option at all. What is the use case for having a persistant tab state across items

avatar tonypartridge
tonypartridge - comment - 2 May 2017

@brianteeman I think its not for across items its designed to be relative to the current selection. But due to the way it has been implemented it is across items.

I originally suggested it should be storing the component as well as the tab state in an object for instance. But as said it's above my JS skillset with the current code implementation as I assumed this would be the quickest way to provide a solution. But we need someone who is familiar with the current setup.

Problem with making it server side now is that 3rd part component developers will then need to do it too, unless they are using JHTML which some are not.

avatar brianteeman
brianteeman - comment - 2 May 2017

@brianteeman I think its not for across items its designed to be relative to the current selection. But due to the way it has been implemented it is across items.

exactly and thats the bug in my opinion although @ggppdk seems to be suggesting that its an option to do that

avatar ggppdk
ggppdk - comment - 2 May 2017

@brianteeman

i dont agree that we need a new option at all

About not having a new option,
how can current view prevent the active TAB being set via "active TAB history" (besides adding extra custom JS code into the view) or is there a way already ?

What is the use case for having a persistant tab state across items

I agree on this one, but it is a sideffect of current way of deciding the active TAB
and it is done by the code that removes &somename=NNNN (&id=NNN) from HREF

why the above was added , i do not know, maybe to avoid making active tab history too large ?

and the above is related to my comment

It seems wrong to me to leave all responsibilty for setting the active TAB into an all-cases handling code

avatar Bakual
Bakual - comment - 2 May 2017

I think the easiest way for now would be to add the item ID to the storage.
That would mean when you add another item you get a clean state again, but when you edit the same item again (doesn't matter how you closed it) you get the last active state.

Imho, there is also another open issue in that the permission tab (group) state isn't stored properly and overrides the form tab. Ideally, a fix would address both issues.

avatar Bakual
Bakual - comment - 2 May 2017

why the above was added , i do not know, maybe to avoid making active tab history too large ?

I also think that could be the reason (the list used to be a very big list), but that isn't an issue anymore since it is now cleared with the session anyway.

avatar tonypartridge
tonypartridge - comment - 2 May 2017

@Bakual surely that's not enough since you access the component i.e. the itemid then edit multiple articles save and new would still render the tab state previously stored.

Would it not need to be store component i.e. com_content followed by the id of the item ?

avatar Bakual
Bakual - comment - 2 May 2017

@tonypartridge I meant the ID of the item (&id=123), not the menu item (&itemid=123) ?

avatar PhilETaylor
PhilETaylor - comment - 2 May 2017

Actually the "id" that is stored should be pinpointing the EXACT set of tabs on the EXACT url...

Its these lines that need improving greatly, maybe with a hash of the full XPATH to the tab elements in the DOM which would provide a unique key for each and every tab use.

avatar dgt41
dgt41 - comment - 2 May 2017

I will ask again what is the exact purpose and the possible use cases of tabset script?
Give me the objective here and I will come up with the appropriate code...

avatar brianteeman
brianteeman - comment - 2 May 2017

@dgt41 as stated on the very first line

Currently we save the tab state so that if you are on the third tab and press save you are not redirected to the first tab.

avatar PhilETaylor
PhilETaylor - comment - 2 May 2017

im working on this - easy fix.

avatar dgt41
dgt41 - comment - 2 May 2017

@brianteeman so this is only for apply?
If so then this script is useless and the functionality should be integrated to toolbar...

avatar PhilETaylor
PhilETaylor - comment - 2 May 2017

the whole (current) concept of saving the tab state is flawed in Joomla 3.7 (and below)

avatar Bakual
Bakual - comment - 2 May 2017

Imho, as said already, it doesn't hurt when it also saves the state for the case you "Save & Close" as long as it is specific to the item/tab/whatever.
If we want (and feel necessary), we can replace the whole script for J4, but I wouldn't do it in J3.

avatar dgt41
dgt41 - comment - 2 May 2017

Removing the id and adding some logic for the permissions tab (or any other tab(s) ) should be ok for 3.x
For J4 it will be moved to the right place: toolbar->apply-button

avatar PhilETaylor
PhilETaylor - comment - 2 May 2017

The tab state should be per set of tabs instance.

If I edit Article 1 and leave it on "Publishing" then next time I open Article 1 I want the tab on Publishing.

If I edit Article 1 and leave it on "Publishing" then next time I open Article 2 I DONT want the tab to start on Publishing. THIS CURRENTLY HAPPENS

At the moment we remove the id from the url, which is used for the storage key. this is easily changed and Im doing that, as well as making it possible to directly link to a tab from a url

avatar PhilETaylor
PhilETaylor - comment - 2 May 2017

Tabs currently is not storing an array like it should so currently in Global Config -> permissions you are two tabs deep, yet only the last tab clicked is stored in the state :(

avatar Bakual
Bakual - comment - 2 May 2017

Tabs currently is not storing an array like it should so currently in Global Config -> permissions you are two tabs deep, yet only the last tab clicked is stored in the state :(

This one is a regression of (I think, not sure) #12503

avatar PhilETaylor
PhilETaylor - comment - 2 May 2017

yes - all kinds of bad :)

avatar ggppdk
ggppdk - comment - 2 May 2017

@PhilETaylor
@Bakual
it is not that PR that is bad, it made a fix that improved things

and whatever is implemented / fixed will be a "best guessing",
there will always be cases that the guessing will fail (e.g. new record forms)

Let's not do a different best guessing when this can be done in a better way

I remember the situation i was trying to convince people that the REL canonical must be done by the component view / code, and not by system plugin that was doing a "best guessing" thing for all component views

At minimum a view should be able to declare that active TABs should not be saved

Maybe @dgt41 would know best way to do this

About TABs depth that is 2nd issue that needs fixing inside tab_state.js (sorry i will not make any PRs, sorry little time to spare currently)

avatar PhilETaylor
PhilETaylor - comment - 2 May 2017

Well Im writing a PR here in the office at 21:13PM - if people like it, test it, and approve it then great - else Ive just wasted my life - again...

avatar brianteeman
brianteeman - comment - 2 May 2017

There is no need to guess anything - thats why the thing is broken in the first place. The only tab that should be stored is that for an existing item.

avatar PhilETaylor
PhilETaylor - comment - 2 May 2017

The only tab that should be stored is that for an existing item.

Some might argue that they also want tabs elsewhere stored, like in the installer, or INSIDE the permissions tab to be stored

avatar brianteeman
brianteeman - comment - 2 May 2017

\nd they would be wrong ;)

avatar PhilETaylor
PhilETaylor - comment - 2 May 2017

well thats what my code will do - it will store the state for every tab widget independently and even if there are multiple tab widgets on a page.

avatar dgt41
dgt41 - comment - 2 May 2017

Maybe @dgt41 would know best way to do this

Well we have script options:

$document->addScriptOptions('js-tabset', array('component' => 'com_foo', 'view' => 'bar', 'layout' => 'foobar', 'id' => 100);

That's to pass all the needed data from php to js, then in the tabset.js recreating the url should be fairly easy

avatar PhilETaylor
PhilETaylor - comment - 2 May 2017

LMAO - did you even look at the JS code?

avatar dgt41
dgt41 - comment - 2 May 2017

@PhilETaylor what do you mean? I wrote some part of this crap...

avatar PhilETaylor
PhilETaylor - comment - 2 May 2017

@brianteeman

Currently we save the tab state so that if you are on the third tab and press save you are not redirected to the first tab.

However the tab state is persistent so if you save and close an article etc on the third tab then when you open a new article etc you are on the third tab.

Would you be happy if when going back to the first article you were still on the third tab?

avatar PhilETaylor
PhilETaylor - comment - 2 May 2017

ok please test #15748 which provides better handling of multiple tab widgets per page (by full xpath) and not saving the state if there is no id in the url (like on the CREATE page of content. giving better future proofing as well

avatar brianteeman brianteeman - change - 2 May 2017
Status Discussion Closed
Closed_Date 0000-00-00 00:00:00 2017-05-02 21:28:34
Closed_By brianteeman
avatar brianteeman brianteeman - close - 2 May 2017
avatar ggppdk
ggppdk - comment - 2 May 2017

LMAO - did you even look at the JS code?

@PhilETaylor , i will answer politely

@dgt41 answered my comment about avoiding guessing,
which is not about just about finding the current URL, but for finding which parts of current URL are important in regards to deciding at server side (component PHP code),

  • which pages can share an active TAB state,
  • and also about disabling the setting of TAB state

and i will not comment on your fix / PR
so that (hopefully) i will not get an answer like the one you gave above

avatar PhilETaylor
PhilETaylor - comment - 2 May 2017

The PHP side has zero knowledge of the tab state at all. It doesnt need to know.

There is even a tabs.js which is not even used which pretends it knows something about storage of state too... used in <3.7.0

Add a Comment

Login with GitHub to post a comment