[FEATURE] Remember last active tab

[a-r-m-i-n]

Relates: #5157

[fdiedler]

@a-r-m-i-n Good feature 👍

[jmsche]

Hi, nice feature :)

Could you please move JS to a JS file? IIRC all JS was recently moved from templates to JS files.

[javiereguiluz]

The problem with this proposal is that the browser history breaks.

Today: I click on a tabbed page, click on different tabs, click on Backspace and I can return to the original page

With this proposal. I click on a tabbed page, click on different tabs, click on Backspace and I go to the previous tab, Backspace again and I go to another tab instead of the previos page

I'd like to have this feature ... but without messing the browser history. Maybe storing the active page in the local browser storage?

[a-r-m-i-n]

Thanks for testing! I think this behaviour can get fixed when pushing the new state to the browser's history (https://github.com/EasyCorp/EasyAdminBundle/pull/5158/files#diff-193b84b4379491300de20548ffdc39f00879b02563ee37144fd79e91fd310ceeR478)

I try to fix this.

Do you want me to outsource the javascript to a separate file, as suggested above?

[a-r-m-i-n]

The problem with this proposal is that the browser history breaks.

I was able to fix this behaviour by reloading the page, on popstate.

[fdiedler]

@javiereguiluz Any news about merging this feature ? Thanks

[michaelKaefer]

Nice feature, some comments:

• Like @jmsche wrote the JS should be moved (it's cleaner, easier to maintain and you don't violate content security policy)
• I would use the count of the tab to identify it in JS code (then the PHP file needs no change to the label's sha1 and we can have two tabs with the same label)
• Why do you use the URL's fragment and not a query parameter tabIndex which is already the case for menuIndex or submenuIndex at some places? Even if you fixed the bug, it doesn't feel right to use the URL's fragment this way and to manipulate the browser history.. If no query parameter then maybe use local storage like proposed in another comment?

[a-r-m-i-n]

@michaelKaefer

Why do you use the URL's fragment and not a query parameter tabIndex

Because the main/sub menu are not provided by javascript. There, each click will produce a new request. This is unnecessary for working with tabs. But when using JS, you need to manipulate the browser's history to make it react like if it were several requests. Fortunately the browser is smart, the refresh will not cause another request to the server.

Your idea regarding the index of each tab, instead of using an identifier based on the label, can cause unexpected behaviour when two people share a link to a specific tab, but the second user has more/less tabs visible, because of different roles. Instead of using the label, we could use an additional identifier, the developer has to define when creating the tab in configuration, in case he/she wants a two tabs with the same label. But I think this is a little bit edge casy.

Using LocalStorage or Cookies makes it impossible to share links to tabs or save those tab as favorite. Storing the position in LocalStorage makes just sense (to me) when each record (e.g. by id) saves its own position. This could be an additional feature. Using the tab id/hash in the URL does not exclude this.

Please give me a hint where to outsource the javascript file.

[michaelKaefer]

@a-r-m-i-n

• Thanks for the explanation, yes an ID is better than using the count.
• Moving the JS is really just removing it from the Twig file an add it somewhere to assets/js/ (if you need to create a new Webpack entry you can take a look at this commit: https://github.com/EasyCorp/EasyAdminBundle/pull/5093/files#diff-1fb26bc12ac780c7ad7325730ed09fc4c2c3d757c276c3dacc44bfe20faf166f) and rebuild the assets before your commit with yarn encore production ; php ./src/Resources/bin/fix-assets-manifest-file.php
• True, shareable links would be good, so no local storage.
• Thinking about UX again it now makes sense that users can navigate the browser history to get back to their last tab.. But wouldn't it be better to use a query parameter with history.pushState(null, '', '/foo?tab=settings'); instead of the URL fragment? It would be unconventional as well but at least we have endless query parameters but only one URL fragment (and the most common use of URL fragments is to scroll to HTML's id attributes).

Good feature anyways, I hope it gets merged.

[a-r-m-i-n]

@michaelKaefer

Using a query param would make this feature much more complex. When you are in the edit view of a record, you already get a bunch of params, like:

• crudAction
• crudControllerFqcn
• entityId
• menuIndex
• submenuIndex
• referrer

Then, you need to parse the whole queryString and ensure that all other params are not touched. And sometimes (not sure when, exactly) you've got the querySignature which prevents you from modifying query parameters on your own.

Also, when I would see ?tab=abc1234 I would expect server-side rendering. Currently, this is not the case. The whole tab functionality is just working on the client-side.

Jumping to anchors (which is the default behaviour of the section in the location) would still work. Just IDs used by the tabs itself would change this behaviour, and enable the tab instead.

[michaelKaefer]

URL signatures are deprecated, and this 2 things would not work with anchors:

• User clicks second tab, then clicks link containing a fragment on the same page and scrolls to the element, then reloads the page and sees the first tab instead of the second
• We cannot share links that contain a tab and also an anchor to scroll to an element at the same time

Anyways, let's wait for other opinions.

[a-r-m-i-n]

The JS could get extended to check for anchors inside of (inactive) tabs and enable the parent tab before jumping to the anchor. 😎

[wehostadm]

@javiereguiluz @a-r-m-i-n The feature is really nice :)

Does it be merged ?

[jmariller]

Hi, and apologies for spamming, but can you provide an update on this topic? I was recently asked to have exactly this feature in one of our projects. Thank you!

[wehostadm]

I really don't understand why anybody anwser for this great feature. If you don't plan to merge it, juste close this PR and say it please or provide an average deadline for merging this :) Thanks

[javiereguiluz]

Thanks a lot for working on this and sorry it took us so long to review and merge this.

I've played with this feature a bit and in #6047 I'm proposing some tweaks and changes. So, I'm closing this one in favor of the new PR. Thanks!