Posted by jucallme on March 14, 2012 at 5:11pm
17 followers
| Project: | Views |
| Version: | 7.x-3.x-dev |
| Component: | User interface |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
| Issue tags: | Needs tests |
Issue Summary
There is a need for the exposed pager to remember its last selects as we have for the exposed filters in views.
Comments
#1
Here is a patch that adds an exposed filter option to the full pages plugin and then saves the selected exposed options in the form to the session of a user for each view, then when they navigate away from the page and back the pager filter then applies the same selections they had previously.
#2
Removed the relic
$display_id = $this->view->current_display;that was doing nothing.#3
The last submitted patch, exposed-pager-remember-last-selection-1482424-2.patch, failed testing.
#4
Fix failed test issue.
#5
queue for testing
#6
Found bug when all is also exposed causes a http error 500 this solves that.
#7
Works great, code looks good. However, it should only be shown when either the offset or items per page are exposed.
Can you add a
#dependencyfor that?#8
amended issue, I split the check box as when the #dependency was of two different parents then it looked like it was popping out of no where in some cases.
#9
So that happens with the replacement patterns in handlers, so maybe we shouldn't worry about it here?
+++ b/plugins/views_plugin_pager_full.incundefined@@ -112,6 +115,15 @@ class views_plugin_pager_full extends views_plugin_pager {
+ '#title' => t('Remember the last selection'),
.
+++ b/plugins/views_plugin_pager_full.incundefined@@ -147,6 +159,16 @@ class views_plugin_pager_full extends views_plugin_pager {
+ '#title' => t('Remember the last selection'),
If not, now these strings need to be differentiated.
#10
Updated descriptions.
#11
queue for testing >> needs review
#12
Can you reroll that as not two commits? That way you can get commit credit.
#13
squashed and rerolled patch
#14
Looking good, some last minute nitpicks, and then I think its ready.
+++ b/plugins/views_plugin_pager_full.incundefined@@ -83,6 +85,7 @@ class views_plugin_pager_full extends views_plugin_pager {
+
Extra line
+++ b/plugins/views_plugin_pager_full.incundefined@@ -353,4 +379,64 @@ class views_plugin_pager_full extends views_plugin_pager {
+ function exposed_form_submit(&$form, &$form_state, &$exclude) {
Could use a docblock
+++ b/plugins/views_plugin_pager_full.incundefined@@ -353,4 +379,64 @@ class views_plugin_pager_full extends views_plugin_pager {
+ * Check to see if input from the exposed pager should change
+ * the behavior of this pager.
Should start with a plural verb, like "Checks". Also, this needs to be a single line, under 80 chars.
+++ b/plugins/views_plugin_pager_full.incundefined@@ -353,4 +379,64 @@ class views_plugin_pager_full extends views_plugin_pager {
+ * Saves input from the exposed pager to the session that will
+ * change the behavior of this pager.
This needs to be a single line, under 80 chars.
+++ b/plugins/views_plugin_pager_full.incundefined@@ -353,4 +379,64 @@ class views_plugin_pager_full extends views_plugin_pager {
+ * Saves input from the exposed pager to the session that will
+ * change the behavior of this pager.
This needs to be a single line, under 80 chars.
#15
adjusted agin.
#16
duno why
+++ b/plugins/views_plugin_pager_full.incundefined@@ -83,6 +85,7 @@ class views_plugin_pager_full extends views_plugin_pager {
+
is reporting with extra line in patch...
#17
is there something holding this patch back?
#18
The store_exposed_input and get_exposed_input look very very similar, except one has checks in $_SESSION and the other has method calls.
Furthermore, store_exposed_input is a method used by handlers, not other plugins, and this one is only called by exposed_form_submit. Why not combine them?
Also, the comment on get_exposed_input should probably be something more like
Allows input from the exposed pager to change the behavior of the pager.The first line of method/function docs have to be under 80 characters.Actually, I've gone ahead and rolled a patch. In the future, please don't use format-patch, it makes it very difficult for a human to parse.
#19
This causes an anonymous session to be set by simply viewing the view display. Do we need to use anonymous sessions for this? This will likely cause breakage with things like varnish that will not work if there is an anonymous session. Could using a cooke instead avoid that? If we really do need the anonymous session, then it should at least only be set if the user has overridden the default value.
#20
This will not work for anyone running a cluster of servers. We have two application servers The users are not forced to either server. One request could go to server a and the next one could go to server b. If they switched servers the $_SESSION variables would not be set. I think using $_COOKIE would avoid this problem.
#21
Moved the cache to use the cookie, also updated this with the other exposed operator.
#22
missed the file.
#23
Not work for me.
#24
We had issues with varnish cache when the views exposed filters are set to remember selection.
Since that option is setting values in session, the varnish cache is not serving pages from cache after that page with exposed filter is visited.
I tried this patch, and it kind of works. The problem is, the cookie works fine when I am on that page. But if I navigate to other pages and come back to this page(with the exposed filters), the cookie value is empty and all my selections are lost.
#25
New features should be backed up with a test.
#26
Any way to get around the cookie issue with the views exposed filters? I'm trying to figure out why Varnish isn't working and thought this might possibly be it. Will setting a cookie resolve this issue?
#27
Hi, I have made a simple module to get the pager remember user options.
Just save the items_per_page variable in $_SESSION and set items_per_page if the option was setted by user.
enjoy,
dropfen
#28
Thanks for posting that module, dropfen - it works! Really helped me out of a bind I was in.
#29
great, thank you for using ;)
#30
Adding some comments to this issue.
@jucallme - Your patch works fine, assuming that all of the modules that inherit or try and read from the $_SESSION variable can edit their code to not read from $_SESSION. I just ran into this today with the date module which reads from $_SESSION for default values.
@mrfelton - is the goal to move everything in views out $_SESSION for exposed filters? If so, that to me is a different issue then the one at hand, which is that we can't remember the items per page filter. If you feel differently let me know.
I'm happy to work on an updated patch but want to know the direction here. Thanks.