Persist the filter for model admins, and pages list view

[glenn-bautista]

Fix: Append the 'q' get variable to the GridField ItemEditForm action, and back link to persist the filter for the model admin, and pages list view.

Found the issue here: silverstripe/silverstripe-cms#503

[sminnee]

This feels like a really heavy-handed way of doing this, and the change of the HTML ID isn't something that I think should be necessary (or included in 3.1).

A few thoughts:

• If you put ?q=XXX onto GridFieldDetailForm::Link(), then it will be included in the FormAction. It could also be included in GridField::Link(), or even ModelAdmin::Link(), and would bubble up.
• It's awkward to hard-code the q get variable into this: we're coupling GridFieldDetailForm to details of the model. If it's not feasible to include the filter parameter in ModelAdmin::Link() (some testing would needed to see the implications of this), then perhaps there should be a method to specify the get variables to carry through. However, that feels like a hack, and despite the pervasiveness of changing ModelAdmin::Link(), it might be the most robust solution.

Would be good to get some views of other devs on this.

[glenn-bautista]

I tried to go with the most unobtrusive code change I could think of to limit the surface area of the change. If you think that that's the best approach, I'd be more than happy to modify the pull request or create a new one.

I think this bug really needs to be fixed, we have a client that has a site with thousands of pages and editing those in the ModelAdmin is horrendous without the filter persisting, at least. We also tried to get the Pagination to persist but could not find a clean (non hacky) way of doing it.

[kinglozzer]

Hey @glenn-bautista. I’ve had a small brainwave here. A possible solution here is to replace the “Back” button with a FormAction that makes use of History.back() (generic behaviour for this was introduced in 556090c). Unfortunately this won’t solve the pagination issue, but at least filters will be preserved.

I’ve done a quick and untidy proof of concept in kinglozzer@1814288, what do you think?

[dhensby]

@kinglozzer nice idea :)

Those damn CSS selectors for behviour are horrendous, though... they should just have a single js hook class that denotes their behaviour...

[stevie-mayhew]

Now that we have 3.2, is this an appropriate solution? The HTML ID of forms have all changed anyway.

I think that this is a bug as it standards, as user expectations are not met with the action on the back button.

[stevie-mayhew]

We've just released a module which does this: https://github.com/Little-Giant/silverstripe-persistentgridfield

[dhensby]

This definitely needs some attention in the core - thanks for the module in the mean-time

[stevie-mayhew]

We'd be happy to port this across to core but the way we've had to do it in the end is really hacky. It definitely requires a bit more thinking about how GridField works internally rather than adding stuff on top.

[gavro]

Created an alternative module to enable some form of states in grid views. Instead of using php+memory/hashes (see @stevie-mayhew) I've tried to get by just using JS utilising sessionStorage. Stateful per tab, state resets when navigating across "modules". See: https://github.com/novatio/silverstripe-statefulgridfield (WIP, developed in SS 3.2).

Any thoughts on this approach? The main issue is that sessionStorage is a client side thing, so the gridviews always fetch the initial listing and right after that the reload (with the previous state) is triggered.

[dhensby]

Why is using the built in GridState not suitable for this?

[gavro]

I wanted to keep track of the state per browser tab and at the same time keep track of "module states" per tab (e.g.: when you navigate from assets/* to settings/* and back again: you'd want the state for any GridViews in assets/* to have been reset and not retain any previously used states... the user is expecting a 'clean slate').

I thought that this would not be possible with GridState and sessionStorage seemed like the way to go. Perhaps I'm mistaken or are you not referring to the JSON encoded data stored in the form itself (in hidden field xxx[GridState]?

[dhensby]

@gavro I'm talking about GridState

You might right that this state will leak across tabs... but I'm not certain - worth a check?

I think implementing sessionStorage would be quite tricky and lead to a lot of overhead about when to clean it up and making sure you don't go over the browsers undefined limits in this area...