DataTables has a "stateSave" option which uses's the browser's local storage to "remember" how someone last changed the configuration of a DataTable: columns settings, filters, etc. This option doesn't work by default because we always load the configuration stored in Columns Settings. I propose to add a function which will check for the existence of a locally-store table configuration before defaults are loaded.
Created attachment 129938 [details] [review] Bug 22276: Add client storage of user-selected DataTables configuration This patch enables use of DataTables' built-in session storage of table configurations. DataTables by default uses the browser's localstorage feature to store the state of each DataTable, including which columns have been hidden by the user. Our table settings system has always overridden this saved state. This patch adds a check for a saved configuration state and if present loads the saved state instead of the default state. The system is enabled in this patch on the libraries administration page. To test, apply the patch and open go to Administration -> Libraries. - Use the "Columns" button to change the visibility setting of multiple columns. - Navigate away from the page and then back to test that the visibility settings have been kept. - Go to Administration -> Table settings -> Administration and change the visibility settings for the "libraries" table in a way that it differs from the visibility settings you set manually. - Return to the libraries administration page and confirm that your manual settings are still being loaded instead of the defaults from table settings.
Created attachment 130548 [details] [review] Bug 22276: Add client storage of user-selected DataTables configuration This patch enables use of DataTables' built-in session storage of table configurations. DataTables by default uses the browser's localstorage feature to store the state of each DataTable, including which columns have been hidden by the user. Our table settings system has always overridden this saved state. This patch adds a check for a saved configuration state and if present loads the saved state instead of the default state. The system is enabled in this patch on the libraries administration page. To test, apply the patch and open go to Administration -> Libraries. - Use the "Columns" button to change the visibility setting of multiple columns. - Navigate away from the page and then back to test that the visibility settings have been kept. - Go to Administration -> Table settings -> Administration and change the visibility settings for the "libraries" table in a way that it differs from the visibility settings you set manually. - Return to the libraries administration page and confirm that your manual settings are still being loaded instead of the defaults from table settings. Signed-off-by: Solène Desvaux <solene.desvaux@biblibre.com>
I have executed the test, it is working for me
We need to choose bug 16881 or this one. See discussion from bug 16881 comment 6.
(In reply to Jonathan Druart from comment #4) > We need to choose bug 16881 or this one. See discussion from bug 16881 > comment 6. Not able to test as branches.pl is borked in master atm. This patch limits the stored settings to a given browser while 16881 appears to store it across browsers. I like the simplicity of this implementation. I feel like we could extend this one to storing the data in a table and restoring to localStorage as a follow-up bug. That would give us the best of both!
Created attachment 133717 [details] [review] Bug 22276: Add client storage of user-selected DataTables configuration This patch enables use of DataTables' built-in session storage of table configurations. DataTables by default uses the browser's localstorage feature to store the state of each DataTable, including which columns have been hidden by the user. Our table settings system has always overridden this saved state. This patch adds a check for a saved configuration state and if present loads the saved state instead of the default state. The system is enabled in this patch on the libraries administration page. To test, apply the patch and open go to Administration -> Libraries. - Use the "Columns" button to change the visibility setting of multiple columns. - Navigate away from the page and then back to test that the visibility settings have been kept. - Go to Administration -> Table settings -> Administration and change the visibility settings for the "libraries" table in a way that it differs from the visibility settings you set manually. - Return to the libraries administration page and confirm that your manual settings are still being loaded instead of the defaults from table settings. Signed-off-by: Solène Desvaux <solene.desvaux@biblibre.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 133718 [details] [review] Bug 22276: (QA follow-up) Add missing USE Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
I've added a follow-up to fix a complaint by the QA script. I think I agree with Kyle here - I like that this seems to work really nicely and is not a big change. I'd still like even more permanent storage, but this here would already be very useful and doesn't seem not too risky at this point in the release cycle. If we can extend it even further as a next step, that would be great too.
Hi, nice indeed. But how will it behave with a change of "Cannot be toggled" columns ? What if a new column is added by upgrade ? Cycle is near the end, I prefer not to push this in 22.05. But I will follow this with interest ;)
It would be great if we could pick this up again. We often get asked about why the settings are not remembered.
Please rebase.
Created attachment 138172 [details] [review] Bug 22276: Add client storage of user-selected DataTables configuration This patch enables use of DataTables' built-in session storage of table configurations. DataTables by default uses the browser's localstorage feature to store the state of each DataTable, including which columns have been hidden by the user. Our table settings system has always overridden this saved state. This patch adds a check for a saved configuration state and if present loads the saved state instead of the default state. The system is enabled in this patch on the libraries administration page. To test, apply the patch and open go to Administration -> Libraries. - Use the "Columns" button to change the visibility setting of multiple columns. - Navigate away from the page and then back to test that the visibility settings have been kept. - Go to Administration -> Table settings -> Administration and change the visibility settings for the "libraries" table in a way that it differs from the visibility settings you set manually. - Return to the libraries administration page and confirm that your manual settings are still being loaded instead of the defaults from table settings. Signed-off-by: Solène Desvaux <solene.desvaux@biblibre.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 138173 [details] [review] Bug 22276: (QA follow-up) Add missing USE Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 22.11. Nice work everyone, thanks!
Enhancement will not be backported to the 22.05.x series
Why do we have this in branches.tt only? We don't want to have one follow-up bug per table, we need to apply it from datatabase.js (or .inc or wherever, but in a single place).
(In reply to Jonathan Druart from comment #16) > Why do we have this in branches.tt only? We don't want to have one follow-up > bug per table, we need to apply it from datatabase.js (or .inc or wherever, > but in a single place). If this is possible, I think we should do it. It would be so nice to have this globally.
(In reply to Katrin Fischer from comment #17) > (In reply to Jonathan Druart from comment #16) > > Why do we have this in branches.tt only? We don't want to have one follow-up > > bug per table, we need to apply it from datatabase.js (or .inc or wherever, > > but in a single place). > > If this is possible, I think we should do it. It would be so nice to have > this globally. See bug 33484