Added persistence in window scope for filtering and tabs state. #921
Conversation
| @@ -19,6 +19,11 @@ function initTabs(){ | |||
| showCorrespondingTabBody(element) | |||
| element.addEventListener('click', (event) => toggleSections(event)) | |||
| }) | |||
| let cached = localStorage.getItem("active-tab") | |||
There was a problem hiding this comment.
Hmm that means that there is only one entry for all pages, so when i change tab on 1 page it will be the same on the others.
I don't think it should work like that. You can add a location to the end of the key to make it page-specific.
Same thing goes for the sourceset filters, but imho persisting them between pages i nice.
There was a problem hiding this comment.
Also you can just do if(cached) and why do you need to JSON.parse it when it is a string?
There was a problem hiding this comment.
I have spoken with @Kordyjan about scope of cache and if I understood correctly, we decided to make filters cache to work between pages.
About comparison - I never know how to check nulls and undefined's correctly.
About JSON.parse - It's true, I didn't realized that it is just string and my default behavior is to call JSON.parse when retrieving data from localStorage.
There was a problem hiding this comment.
Fixed null-checks
| if (cached !== null) { | ||
| let parsed = JSON.parse(cached) | ||
| filteringContext.activeFilters = filteringContext.restrictedDependencies | ||
| .filter(q => -1 == parsed.indexOf(q)) |
There was a problem hiding this comment.
i find it weirdly disturbing to have a reversed equals XD
| let cached = localStorage.getItem("active-tab") | ||
| if (cached !== null) { | ||
| let parsed = JSON.parse(cached) | ||
| document.querySelector("div[tabs-section] > button[data-togglable=" + parsed + "]").click() |
There was a problem hiding this comment.
Small thing but this will cause an error when on the first page we click on a tab and then move to another page that doesn't have this tab.
80c96a2 to
47ebe8f
Compare
#917