Even more search improvements - 2nd attempt :)

sparkica commented 9 years ago

I moved search button to the top,
added bigger More button,
did some cleaning (removed unused code),
replaced search POST request with GET request: search url gets updated and can be bookmarked,
fixed pagination (no more custom endless js needed) and moved the text displaying number of results,
introduced history.js due to IE issues (https://github.com/browserstate/history.js),
replaced search parameter with q, so it is consistent with clickable searchable tags.

Sorry about creating a second PR, but the first one was a mess. @gandalfar please take a look at this one.

coveralls commented 9 years ago

Coverage increased (+0.05%) when pulling 7ff12f2184340d928ec075b77cb8ec9cf13818d2 on sparkica:fea_search_get2 into cbcd4ae290cbb7c41071d269643bea55f44ce330 on codeeu:master.

coveralls commented 9 years ago

Coverage increased (+0.55%) when pulling dceef98a246f7dfc53874b71db4a46353cbbab50 on sparkica:fea_search_get2 into cbcd4ae290cbb7c41071d269643bea55f44ce330 on codeeu:master.

coveralls commented 9 years ago

Coverage increased (+0.54%) when pulling dd1d4cc90283597bbbe65ccf70d1942912036d21 on sparkica:fea_search_get2 into cbcd4ae290cbb7c41071d269643bea55f44ce330 on codeeu:master.

ialja commented 9 years ago

Looking good, great job! :+1:

A few minor comments:

On the index page, we started using past=yes for the past filter, the new search uses past=on. Can you please switch back to past=yes for consistency?
As I'm using the real events website, I keep getting the feeling that we could display more search results per page. Maybe increase the number from 6 to 10?
Any special reason why you switched from "X events match your search criteria:" to "Events matching your search criteria: X"? I kinda feel the first wording is less robotic :)

sparkica commented 9 years ago

Regarding 'past=yes' @gandalfar will have an answer, I don't :)
From 6-10: sure, no problem.
Yes, I agree it was less robotic. I changed it because number of results found has to be updated with every AJAX call (every click on any of the search criteria) via the script and it made more sense to just update the number itself rather than whole wording. However... this can be changed :smiley_cat:

ialja commented 9 years ago

We've already shared links with past=yes, so it might be better to keep it that way :)

jurecuhalev commented 9 years ago

I think it was something with serialization, but no problem @sparkica can easily change that constant.

sparkica commented 9 years ago

@gandalfar Not quite sure what you're asking. Take a look at the History repo, this is from their documentation:

Provide a backwards-compatible experience for all HTML4 Browsers using a hash-fallback (including continued support for the HTML5 History API's data, title, pushState and replaceState) with the option to remove HTML4 support if it is not right for your application
Provide a forwards-compatible experience for HTML4 States to HTML5 States (so if a hash-fallbacked url is accessed by a HTML5 browser it is naturally transformed into its non-hashed url equivalent)

So if I understand correctly, we don't need any extra JS code to handle hashbangs.

coveralls commented 9 years ago

Coverage increased (+0.54%) when pulling 9317cb66d153e6d87b5bad8ef5014dc9a61db12a on sparkica:fea_search_get2 into cbcd4ae290cbb7c41071d269643bea55f44ce330 on codeeu:master.

jurecuhalev commented 9 years ago

@sparkica try this url in IE9: /search/#?q=&country=SI&past_events=on and you'll see that you don't get correct results because hashbang part of URL is not sent to server.

See detailed list of issues with hashbangs from their URL: https://github.com/browserstate/history.js/wiki/Intelligent-State-Handling

I would propose to drop html4 history rewriting support and either do full refresh of page url or not bother with history rewriting for old browsers.

sparkica commented 9 years ago

@gandalfar Thanks for clarification (I don't have access to IE9 right now), I'll will figure something out.

jurecuhalev commented 9 years ago

@sparkica download IE9 from https://www.modern.ie/en-us

sparkica commented 9 years ago

Working on it :+1: Did a little research.

coveralls commented 9 years ago

Coverage increased (+0.54%) when pulling d8841cb5703e51c8aafd1a9a03142d69c534bbab on sparkica:fea_search_get2 into cbcd4ae290cbb7c41071d269643bea55f44ce330 on codeeu:master.

sparkica commented 9 years ago

@ialja I've replaced all the past='yes' values to past=on. When using GET and Django forms checkboxes, True is converted to 'on' and False to 'off' by default. This way we can avoid serialization issues.

coveralls commented 9 years ago

Coverage increased (+0.73%) when pulling a85457b2a8908c9cf854fbfde5396fd22a0d2ef7 on sparkica:fea_search_get2 into cbcd4ae290cbb7c41071d269643bea55f44ce330 on codeeu:master.

coveralls commented 9 years ago

Coverage increased (+1.13%) when pulling d40d9414159bc61162fd64985e3c898b19fa1556 on sparkica:fea_search_get2 into cbcd4ae290cbb7c41071d269643bea55f44ce330 on codeeu:master.

sparkica commented 9 years ago

Well... it seems we already have modernizr included, I'll use it then.

coveralls commented 9 years ago

Coverage increased (+1.2%) when pulling 62e9fe137c0e55d90272cad0c2ffaeef1679b826 on sparkica:fea_search_get2 into cbcd4ae290cbb7c41071d269643bea55f44ce330 on codeeu:master.

coveralls commented 9 years ago

Coverage increased (+1.2%) when pulling 62e9fe137c0e55d90272cad0c2ffaeef1679b826 on sparkica:fea_search_get2 into cbcd4ae290cbb7c41071d269643bea55f44ce330 on codeeu:master.

coveralls commented 9 years ago

Coverage increased (+1.24%) when pulling 27782fd198d41dc61697a40f8f573fc081a77528 on sparkica:fea_search_get2 into cbcd4ae290cbb7c41071d269643bea55f44ce330 on codeeu:master.

goranche commented 9 years ago

this was reverted, because of bugs... please create a new PR (found issues will be written)

codeeu / coding-events

Even more search improvements - 2nd attempt :) #359