Hi,

I added support for the HTML5 history API to update the URL when facets are clicked as well as history support so people can use the 'back' button in their browser.

NOTE: This patch needs to be applied after this one (for Facet API Pretty Paths support)

Thanks!
drclaw

UPDATE: patch #35 ONLY for 7.x-3.4 branch
Patch that adds support of browser history (for 7.x-3.4 branch)

Check status report page to get more info.
1. If there is not installed libraries module: https://sc-cdn.scaleengine.net/i/36b0658cdc4782a0b7beef06aa2dfa87.png
2. If libraries module installed but history.js not found: https://sc-cdn.scaleengine.net/i/b8cf0e9d8eb87546204e7421b186aa9c.png
3. If libraries module installed and history.js found: https://sc-cdn.scaleengine.net/i/c3812a240630c51bcba6908e90050c18.png
Thanks!
Loparev

Comments

eugene.ilyin’s picture

Status: Active » Postponed

This task is postponed

eugene.ilyin’s picture

Category: Feature request » Plan
Issue summary: View changes
rich.3po’s picture

Hi - just wondering if theres been any update on this one? I'm trying to make filter-enabled pages linkable and bookmarkable (ie RESTful), so that they can be linked to with pre-enabled filters from other content pages

The back-browsing functionality would also be nice, but not essential in my case

thanks

sachbearbeiter’s picture

+1 would be really useful to have ...
otherwise you have to open the full nodes via ajax or in a new window ...

umakart’s picture

For a search with more than 10 facets each with lot of options the back-browsing functionality would be really important. Please!

eugene.ilyin’s picture

Seems it's important feature. I'll try to find time to do it.

eugene.ilyin’s picture

Status: Postponed » Active
idebr’s picture

Ideally this feature would be compatible with Views ajax history or even let that module do the heavy lifting.

idebr’s picture

Status: Active » Needs review
FileSize
784 bytes

Attached patch adds integration with the Views ajax history. For this patch to work correctly, you also need the patch from #2610240: Allow other modules to use their own ajax urls

gunwald’s picture

Patch in #9 does not work for me.

Attached patch adds integration with the Views ajax history. For this patch to work correctly, you also need the patch from #2610240: Allow other modules to use their own ajax urls

I did as suggested, both patches were applied cleanly, but can't see any effect.

Loparev’s picture

Hi. I have "ported" this patch from @drclaw for 7.x-3.4 (maybe it can be applied to 7.x-3.x dev).

Now all successful ajax requests from facets pushes into html5 browser history. And if click on back button then view would be refreshed with previous selected facets. It works for me.

Could someone check and review it?

Loparev’s picture

Assigned: Unassigned » Loparev
gunwald’s picture

Seems to work as expected. I could apply patch to 7.x-3.4:

git apply -v ajax_facets-html5-states-2105177-11.patch
Checking patch misc/ajax_facets.js...
warning: misc/ajax_facets.js has type 100755, expected 100644
Applied patch misc/ajax_facets.js cleanly.

Facets are pushed to history updating the URL.

Thank you! Please commit this patch as soon as possible.

Loparev’s picture

I have improved patch: for now it uses history.js library. It means that ajax history functionality will work in html4 browsers. But it also means that ajax_facets module has dependencies: libraries module.

So, before applying this patch you need to:
1. Download and install libraries module.
2. Download and unpack history js library to sites/all/libraries/history.js (or another libraries) directory.

I think it is justified to use libraries module and history.js library. Because for now there is a lot of html4 browsers in which history feature does not work.

Here is the patch.

Loparev’s picture

Sorry, there are some errors in previous patch. Fixed.

Loparev’s picture

Version: 7.x-2.x-dev » 7.x-3.4
BR0kEN’s picture

+++ b/misc/ajax_facets.js
@@ -543,10 +543,22 @@
+    $(window).unbind('statechange', Drupal.ajax_facets.reactOnStateChange);

Do you really need to wrap window object into jQuery twice per function call? I guess the invocation of $(window) could be moved above the function.

Loparev’s picture

You are right. There should be

var $window = $(window);

and then

$window.bind(...);
...
$window.unbind(...);
Loparev’s picture

  1. Code refactoring
  2. @BR0kEN review fix
  3. Fixed bug with initial binding to "statechange" event
Loparev’s picture

Loparev’s picture

Issue summary: View changes
Loparev’s picture

Issue summary: View changes
Loparev’s picture

FileSize
8.45 KB
354 bytes

Sorry, one problem was found in previous patch. Fixed.

Loparev’s picture

Issue summary: View changes
Loparev’s picture

FileSize
11.25 KB
8.93 KB

it's me again)

Finally, I've done (I hope) work on this patch.

For now this feature works fine without any hard dependencies to libraries module and history.js library. It means that if history.js library available then it would be used. If not - HTML5 history API would be used. If browser does not support HTML5 history API then nothing would happen (like default ajax_facets behaviour).

So, here is a final patch.

Loparev’s picture

@gunwald, could you check this patch and confirm all works as expected without any problems?

Loparev’s picture

Issue summary: View changes
Loparev’s picture

Issue summary: View changes
Loparev’s picture

Issue summary: View changes
umakart’s picture

@Loparev Many, many thanks for your work. I apply your latest patch(#25) and everything act as expected.
This functionality is an urgent need for every search page with ajax_facets. Please commit this patch as soon as possible!

gunwald’s picture

I tired to apply patch in #25 (on fresh 7.x-3.4), but got the following error:

ajax_facets# git apply -v ajax_facets-html5-states-history-js-2105177-25.patch
Checking patch ajax_facets.install...
error: ajax_facets.install: No such file or directory
Checking patch ajax_facets.module...
warning: ajax_facets.module has type 100755, expected 100644
Checking patch misc/ajax_facets.js...
warning: misc/ajax_facets.js has type 100755, expected 100644

After putting an empty "ajax_facets.install" in the folder, the patch applies.

Loparev’s picture

@gunwald, try
patch -p1 < ajax_facets-html5-states-history-js-2105177-25.patch

I suppose git apply can't create new file

Loparev’s picture

Unfortunately one more bug was found in this patch.

If you have additional query parameters on your search page and if you click on facets then that additional parameters would be replaced by facets parameters.

Steps to reprpduce:
1. Load search page with some query parameter: http://your.site/search-view?some-aditional-parameter=test
2. Click on some facets.
3. Your some-aditional-parameter would be replaced by facet parameters: http://your.site/search-view?clicked-facet-id=value

This problem was fixed in this patch.

Loparev’s picture

Issue summary: View changes
Loparev’s picture

// Facetapi module has a bug when facet name encodes twice.
    // For example to get this facet work 'category:name:pineapple' it should be 'category%253Aname%3Apineapple'.
    // It means that first ':' was encoded twice. Why we don't patch facetapi? Because a lot of sites have already
    // used facetapi module and they links (with wrong encoded facets names) has been indexed by search engines
    // like Bing or Google. So we just bring this behaviour to ajax_facets module.
    // Encode each facet filter name (it have already encoded once in FacetapiAjaxWidgetCheckboxes::buildListItems()).
Loparev’s picture

Issue summary: View changes

  • eugene.ilyin committed afa6d2f on 7.x-3.x authored by Loparev
    Issue #2105177 by Loparev, idebr, drclaw, eugene.ilyin: Update URL and...
eugene.ilyin’s picture

Status: Needs review » Needs work

@Loparev, thank you for your work. It's really great efforts.
I've applied your patch with small corrections.
But I've found a bug. If I'll select some options for facets, then results will be updated. But if I'll go back via browser buttons, the results and facets will not be updated. I have no history.js but I don't think that it needs for my chrome 47.

Could you help with it?

Loparev’s picture

FileSize
14.77 KB
14.97 KB

Hi @eugene.ilyin.

I think I found problem.

You changed patch in that way that Drupal.ajax_facets.initHistoryState($this); calls from Drupal.ajax_facets.sendAjaxQuery. But it's incorrect because we want init history only when user click on some kind of facets (checkbox, select, slider, link). And Drupal.ajax_facets.sendAjaxQuery calls from Drupal.ajax_facets.reactOnStateChange too and it's wrong.

So to resolve this issue you need to remove call Drupal.ajax_facets.initHistoryState($this); from Drupal.ajax_facets.sendAjaxQuery and put it to all Drupal.ajax_facets.process* functions (as it was). It works for me.

Compare revert_this.patch and apply_this.patch (I've applied patch from your commit afa6d2f against 3.4 version to create this diff so in apply_this.patch exists some extra changes. But try to focus on Drupal.ajax_facets.sendAjaxQuery and Drupal.ajax_facets.initHistoryState($this); differences).

Please let me know if it doesn't work for you.

(By the way. I've tested it against 7.x-3.4 version of ajax_facets module as I've mentioned it in #11 comment. But I believe it should works for dev too)

eugene.ilyin’s picture

Hi @loparev, I've committed the changes. Seems it's solves problem that you've mentioned. But it's still not working. Could you check the reason of this trouble?

Loparev’s picture

Hi.

1. Install history.js and check it again. Is it works now?
2. Try in another browser. Still not working?
3. install 3.4 ajax_facets and apply patch 35. Does it work for you? If it works for 3.4 with patch from #35 we need port this patch for 7.x-3.x-dev.

It strange. I'll check it tonight.

eugene.ilyin’s picture

1. Install history.js and check it again. Is it works now?
2. Try in another browser. Still not working?

I'll try to check it tomorrow

3. install 3.4 ajax_facets and apply patch 35. Does it work for you? If it works for 3.4 with patch from #35 we need port this patch for 7.x-3.x-dev.

Sorry, but all patches should be for dev versions. Now we have pretty big difference between 3.4 and 3.x-dev.
When I want to check some patch, I always applying it for dev version.

Loparev’s picture

Oops and oops

Some misunderstanding.

Ok, I'll "port" patch for dev branch as soon as possible. I'm sure it is not so hard.

Loparev’s picture

Yeah, it's my fault.

I work on a project and we use stable release of ajax_facets and I've wrote patch for stable release.

You're right, patch should be applyeble to dev.

eugene.ilyin’s picture

Version: 7.x-3.4 » 7.x-3.x-dev

Yes, it should be 7.x-3.x-dev.

So I've already pushed this half working patch into dev branch to avoid some delays with applying this functionality. And we don't have back way :D
Could you just prepare an additional corrections for 7.x-3.x-dev, but not new patch from scratch?

Loparev’s picture

but not new patch from scratch?

Sure)

Loparev’s picture

Here is a fix for 7.x-3.x-dev branch. I hope it helps)

Loparev’s picture

Status: Needs work » Needs review
Loparev’s picture

Issue summary: View changes
Loparev’s picture

  • eugene.ilyin committed 8b4f599 on 7.x-3.x authored by Loparev
    Issue #2105177 by Loparev, idebr, drclaw, eugene.ilyin, gunwald, umakart...
eugene.ilyin’s picture

Status: Needs review » Needs work

Well, it works for me with the history.js. Seems this library is required because of my doctype is for html 4.

I've found another error:
1. Find something with exposed form (by ajax) - I have 2 results.
2. I should click on the facet option (checkboxes) twice to activate it.

Loparev’s picture

I'll check it

Loparev’s picture

Status: Needs work » Needs review
FileSize
1.24 KB

First.

Well, it works for me with the history.js. Seems this library is required because of my doctype is for html 4.

Works for me.
Status report: https://screencloud.net/v/df8S
Browser: https://screencloud.net/v/41AJ
May be it's really because of your doctype.

Second.

I've found another error:
1. Find something with exposed form (by ajax) - I have 2 results.
2. I should click on the facet option (checkboxes) twice to activate it.

This issue related to Trouble with tooltip and to Title of the facet option in widget "Ajax multiple checkboxes" isn't clickable

Look at chronology of events:
1. First you had issue "Trouble with tooltip" where you've removed #id from checkboxes in this commit

diff --git a/plugins/facetapi/ajax_widget_checkboxes.inc b/plugins/facetapi/ajax_widget_checkboxes.inc
index cac5d0f..14c335b 100755
--- a/plugins/facetapi/ajax_widget_checkboxes.inc
+++ b/plugins/facetapi/ajax_widget_checkboxes.inc
@@ -94,6 +94,7 @@ class FacetapiAjaxWidgetCheckboxes extends FacetapiAjaxWidget {
         'data-facet-value' => $value,
         'data-facet-name' => rawurlencode($this->settings->facet),
         'data-raw-facet-name' => $this->settings->facet,
+        'data-facet-uuid' => 'ajax-facets-checkboxes-' . str_replace(array('_', ' ', ':'), '-', $this->key . '-' . drupal_strtolower($value)),
       );
       // Respect current selection.
       if (!empty($item['#active'])) {
@@ -104,7 +105,6 @@ class FacetapiAjaxWidgetCheckboxes extends FacetapiAjaxWidget {
       }
 
       $checkbox = array(
-        '#id' => 'ajax-facets-checkboxes-' . str_replace(array('_', ' ', ':'), '-', $this->key . '-' . drupal_strtolower($value)),
         '#name' => rawurlencode($this->key),
         '#type' => 'checkbox',
         '#title' => $item['#markup'] . ' ' . theme('facetapi_count',(array('count' => $item['#count']))),

2. And then you've got another problem "Title of the facet option in widget "Ajax multiple checkboxes" isn't clickable". It's because of #id attribute in form element. Without #id Drupal renders element title like <label> html element but without for attribute (if user clicks on label with for attribute then event for input would be triggered automatically without any js). And you've commited this patch

diff --git a/misc/ajax_facets.js b/misc/ajax_facets.js
index 1834494..a7aba3d 100755
--- a/misc/ajax_facets.js
+++ b/misc/ajax_facets.js
@@ -101,6 +101,13 @@
               [settings.facetapi.facets[index]],
               Drupal.ajax_facets.processCheckboxes
             ).addClass('processed');
+
+            // Make label of the checkbox clickable.
+            $('#' + settings.facetapi.facets[index].id + '-wrapper ul li').click(
+              function () {
+                $(this).find('input').trigger('click');
+              }
+            );
           }

This is the real problem. Because checkbox input triggers a lot of times because of that js (even if you click on input that js would trigger another one event). I suppose it's not ok and we should use default behaviour with for attribute for label.

This patch will help. But if you will get "Trouble with tooltip" issue again I think it's really another bug and it doesn't related to history issue and you should fix it in another thread.

Loparev’s picture

  • eugene.ilyin committed 20ae8c4 on 7.x-3.x authored by Loparev
    Issue #2105177 by Loparev, idebr, drclaw, eugene.ilyin, gunwald, umakart...
eugene.ilyin’s picture

Status: Needs review » Fixed

Yes, you're right. Now all works fine. Thank you for this great job.
I think that we can close the issue.

If somebody will have troubles with this functionality, please create separate issue. But would be useful to relate the new issue with this one.

eugene.ilyin’s picture

Status: Fixed » Needs work

Sorry, I forgot about small but important thing. Could you prepare info for README.txt and for page of module to let users know about supporting of History API. Just in two words.

Loparev’s picture

Yes, sure.

Loparev’s picture

FileSize
974 bytes

You can add this info to project page

Module has integration with HTML5 browser history API. If you want to get this feature work for HTML4 browsers you should install libraries module (https://www.drupal.org/project/libraries) and download history.js (https://github.com/browserstate/history.js) library.

sachbearbeiter’s picture

From me also: "Thank you for this great job."

eugene.ilyin’s picture

Status: Needs work » Fixed

Thank you all guys!

gunwald’s picture

I tried the actual dev, but it does not work at all. I get the following JS error:

Uncaught TypeError: Cannot read property 'length' of undefinedb.extend.each @ jquery-1.9.1.min.js:3Drupal.ajax_facets.getFacetValues @ ajax_facets.js?o2t0vd:386Drupal.ajax_facets.bindResetLink @ ajax_facets.js?o2t0vd:181Drupal.behaviors.ajax_facets.attach @ ajax_facets.js?o2t0vd:96(anonymous function) @ drupal.js?o2t0vd:76b.extend.each @ jquery-1.9.1.min.js:3Drupal.attachBehaviors @ drupal.js?o2t0vd:74(anonymous function) @ authcache_p13n.js?o2t0vd:29b.extend.each @ jquery-1.9.1.min.js:3(anonymous function) @ authcache_p13n.js?o2t0vd:28b.extend.each @ jquery-1.9.1.min.js:3b.fn.b.each @ jquery-1.9.1.min.js:3$.fn.authcacheP13nReplaceWith @ authcache_p13n.js?o2t0vd:9(anonymous function) @ authcache_ajax.js?o2t0vd:56b.extend.each @ jquery-1.9.1.min.js:3b.fn.b.each @ jquery-1.9.1.min.js:3(anonymous function) @ authcache_ajax.js?o2t0vd:55(anonymous function) @ authcache_ajax.js?o2t0vd:28b.extend.each @ jquery-1.9.1.min.js:3$.ajax.success @ authcache_ajax.js?o2t0vd:27c @ jquery-1.9.1.min.js:3p.fireWith @ jquery-1.9.1.min.js:3k @ jquery-1.9.1.min.js:5r @ jquery-1.9.1.min.js:5

The page reloads when I click on a Ajax facet.

eugene.ilyin’s picture

Status: Fixed » Needs work
Loparev’s picture

Why needs work? It worked. Are you serious?. There was two or three commits after this commit. May be lets look in to that commits? There was js refactoring.

eugene.ilyin’s picture

Please don't panic :) I just afraid that if it will be with status "fixed" we will loose this issue.
Maybe better is create the new issue.

@gunwald, could you do it?

Loparev’s picture

Yep. Create a new issue and relate it to this one.

Loparev’s picture

@gunwald, the latest dev works fine for me. I have jQuery 1.10.2 on my page with search view. Could you provide more details?

Loparev’s picture

Any updates?

Loparev’s picture

@gunwald, did you find the problem?

eugene.ilyin’s picture

Status: Needs work » Closed (fixed)

I think we close this issue.
@anybody, please do not open this issue again. If you will have troubles, please report them into the new issue.

jagermonster’s picture

Great work, tested and working