Fix search history #748

aphillipo · 2017-07-02T09:53:30Z

Well this was a battle! Do we support browsers that don't have the History API?

Please try to break this obviously, back buttons can be rather awkward to get right!

Fixes #341

sourcelevel-bot · 2017-07-02T09:54:04Z

assets/js/search.js

+export function popstateHandler (event) {
+  if(event.originalEvent.state == null) {
+    // NOTE: for reasons only known to the browser makers we need to reload here, 
+    //       on back after navigating away the page (clicking a result in the search) 


Trailing spaces not allowed.

sourcelevel-bot · 2017-07-02T09:54:05Z

assets/js/search.js

-  search(sidebarNodes, searchVal)
+export function popstateHandler (event) {
+  if(event.originalEvent.state == null) {
+    // NOTE: for reasons only known to the browser makers we need to reload here, 


Trailing spaces not allowed.

sourcelevel-bot · 2017-07-02T09:54:06Z

assets/js/search.js


-  search(sidebarNodes, searchVal)
+export function popstateHandler (event) {
+  if(event.originalEvent.state == null) {


Expected space(s) after "if".

sourcelevel-bot · 2017-07-02T09:54:06Z

assets/js/search.js

  })
 }

+function closeResults(e, removeResults) {


Missing space before function parentheses.

sourcelevel-bot · 2017-07-02T09:54:06Z

assets/js/search.js


-  $.merge($results.find('a'), $sidebarItems).on('click', closeResults)
+  // every other link closes the search 
+  $.merge($results.find('a').not('.close-search'), $sidebarItems).on('click', function(e) {


Missing space before function parentheses.

sourcelevel-bot · 2017-07-02T09:54:07Z

assets/js/search.js

  })

-  $.merge($results.find('a'), $sidebarItems).on('click', closeResults)
+  // every other link closes the search 


Trailing spaces not allowed.

sourcelevel-bot · 2017-07-02T09:54:07Z

assets/js/search.js

-    })
+  // sometimes we need to stop adding to the history if it already exists.
+  if (addHistory !== false) {
+    history.pushState({searchValue: value}, "Searching for " + value, "/Search.html?q=" + value);


Extra semicolon.

Read more about it here.

sourcelevel-bot · 2017-07-02T09:54:07Z

assets/js/search.js

-    })
+  // sometimes we need to stop adding to the history if it already exists.
+  if (addHistory !== false) {
+    history.pushState({searchValue: value}, "Searching for " + value, "/Search.html?q=" + value);


Strings must use singlequote.

Read more about it here.

sourcelevel-bot · 2017-07-02T09:54:07Z

assets/js/search.js

-    })
+  // sometimes we need to stop adding to the history if it already exists.
+  if (addHistory !== false) {
+    history.pushState({searchValue: value}, "Searching for " + value, "/Search.html?q=" + value);


Strings must use singlequote.

Read more about it here.

sourcelevel-bot · 2017-07-02T09:54:08Z

assets/js/search.js

+  return decodeURIComponent(results[2].replace(/\+/g, " "));
+}
+
+function pushLevel (levels, search_entity, name) {


Identifier 'search_entity' is not in camel case.

Enforce the usage of camel case to have a consistent codebase.

Read more about it here.

sourcelevel-bot · 2017-07-02T09:54:08Z

assets/js/search.js

+      results = regex.exec(url);
+  if (!results) return null;
+  if (!results[2]) return '';
+  return decodeURIComponent(results[2].replace(/\+/g, " "));


Extra semicolon.

Read more about it here.

sourcelevel-bot · 2017-07-02T09:54:08Z

assets/js/search.js

+      results = regex.exec(url);
+  if (!results) return null;
+  if (!results[2]) return '';
+  return decodeURIComponent(results[2].replace(/\+/g, " "));


Strings must use singlequote.

Read more about it here.

sourcelevel-bot · 2017-07-02T09:54:09Z

assets/js/search.js

+  var regex = new RegExp("[?&]" + name + "(=([^&#]*)|&|#|$)"),
+      results = regex.exec(url);
+  if (!results) return null;
+  if (!results[2]) return '';


Extra semicolon.

Read more about it here.

sourcelevel-bot · 2017-07-02T09:54:09Z

assets/js/search.js

+  name = name.replace(/[\[\]]/g, "\\$&");
+  var regex = new RegExp("[?&]" + name + "(=([^&#]*)|&|#|$)"),
+      results = regex.exec(url);
+  if (!results) return null;


Extra semicolon.

Read more about it here.

sourcelevel-bot · 2017-07-02T09:54:09Z

assets/js/search.js

+  if (!url) url = window.location.href;
+  name = name.replace(/[\[\]]/g, "\\$&");
+  var regex = new RegExp("[?&]" + name + "(=([^&#]*)|&|#|$)"),
+      results = regex.exec(url);


Extra semicolon.

Read more about it here.

sourcelevel-bot · 2017-07-02T09:54:09Z

assets/js/search.js

+  if (!url) url = window.location.href;
+  name = name.replace(/[\[\]]/g, "\\$&");
+  var regex = new RegExp("[?&]" + name + "(=([^&#]*)|&|#|$)"),
+      results = regex.exec(url);


Expected indentation of 4 spaces but found 6.

Read more about it here.

sourcelevel-bot · 2017-07-02T09:54:10Z

assets/js/search.js

+export function getParameterByName(name, url) {
+  if (!url) url = window.location.href;
+  name = name.replace(/[\[\]]/g, "\\$&");
+  var regex = new RegExp("[?&]" + name + "(=([^&#]*)|&|#|$)"),


Strings must use singlequote.

Read more about it here.

sourcelevel-bot · 2017-07-02T09:54:10Z

assets/js/search.js

+export function getParameterByName(name, url) {
+  if (!url) url = window.location.href;
+  name = name.replace(/[\[\]]/g, "\\$&");
+  var regex = new RegExp("[?&]" + name + "(=([^&#]*)|&|#|$)"),


Strings must use singlequote.

Read more about it here.

sourcelevel-bot · 2017-07-02T09:54:10Z

assets/js/search.js

+export function getParameterByName(name, url) {
+  if (!url) url = window.location.href;
+  name = name.replace(/[\[\]]/g, "\\$&");
+  var regex = new RegExp("[?&]" + name + "(=([^&#]*)|&|#|$)"),


Split initialized 'var' declarations into multiple statements.

sourcelevel-bot · 2017-07-02T09:54:10Z

assets/js/search.js

-function search (nodes, value) {
+export function getParameterByName(name, url) {
+  if (!url) url = window.location.href;
+  name = name.replace(/[\[\]]/g, "\\$&");


Extra semicolon.

Read more about it here.

aphillipo · 2017-07-02T10:03:09Z

Okay lot's of little things to fix here...

sourcelevel-bot · 2017-07-02T10:14:54Z

assets/js/search.js

 }

-function search (nodes, value) {
+export function getParameterByName(name, url) {


Missing space before function parentheses.

sourcelevel-bot · 2017-07-02T10:14:54Z

assets/js/events.js

+  $(window).on('popstate', popstateHandler)
+
+  // if the search stub is refreshed or loaded perform the search.
+  if (window.location.pathname == '/Search.html') {


Bad equality comparison: Expected '===' and instead saw '=='.

aphillipo · 2017-07-02T10:17:38Z

Okay Ebert should be happy now!

josevalim · 2017-07-02T10:26:36Z

I am not the person to review this one. :) So let's ping @nscyclone, @dignifiedquire, @milmazz and @eksperimental.

sourcelevel-bot bot reviewed Jul 2, 2017

View reviewed changes

assets/js/search.js Outdated

})

}

function closeResults(e, removeResults) {

Copy link

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space before function parentheses.

sourcelevel-bot bot reviewed Jul 2, 2017

View reviewed changes

aphillipo force-pushed the master branch from 95b7c9d to 336d603 Compare July 2, 2017 10:14

sourcelevel-bot bot reviewed Jul 2, 2017

View reviewed changes

aphillipo force-pushed the master branch from 336d603 to fc75b47 Compare July 2, 2017 10:17

aphillipo force-pushed the master branch from fc75b47 to 2eb00a1 Compare July 2, 2017 10:23

aphillipo mentioned this pull request Jul 2, 2017

[Search] Support CTRL+(ENTER/Click) #533

Closed

aphillipo closed this Jul 2, 2017

aphillipo force-pushed the master branch from 2eb00a1 to 29a243c Compare July 2, 2017 10:28

aphillipo mentioned this pull request Jul 2, 2017

Fix search history #749

Merged

Fix search history #748

Fix search history #748

Uh oh!

Conversation

aphillipo commented Jul 2, 2017

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

aphillipo commented Jul 2, 2017

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

aphillipo commented Jul 2, 2017

Uh oh!

josevalim commented Jul 2, 2017

Uh oh!

Uh oh!