boakley / robotframework-hub

Web app for accessing robot framework assets
https://github.com/boakley/robotframework-hub/wiki
Apache License 2.0
170 stars 60 forks source link

Incremental search #23

Closed yanne closed 10 years ago

yanne commented 10 years ago

Prevent page load when doing a search in the documentation page. The search is done incrementally as the user types, with a debounce of 200ms.

The layout is changed so that the library listing is shown also with the search results.

Currently, there's no way to get back to the initial doc page other than manually reloading the page, should there be?

boakley commented 10 years ago

Very cool. It's probably OK that you can't get back to the initial doc page, though did you consider jumping back to it if the search string was blank?

Out of curiosity, how did you decide on a 200ms bounce time?

boakley commented 10 years ago

I noticed that if I search for something, click on a link in the search results, and then click the browser back button, it does not take me back to the search results. Can that be fixed?

yanne commented 10 years ago

though did you consider jumping back to it if the search string was blank?

This would be quite easy to do, I think. I'll do it and let's see how it feels.

Out of curiosity, how did you decide on a 200ms bounce time?

That's a sensible default I've used elsewhere :) It's always a tradeoff between server load and user experience, but 200ms is little enough that users don't experience it as delay.

I noticed that if I search for something, click on a link in the search results, and then click the browser back button, it does not take me back to the search results. Can that be fixed?

Yes, sure, I'll take a look into that.

yanne commented 10 years ago

I added the history so that using back button now does the search again.

boakley commented 10 years ago

There still seems to be something wrong. If I type "should", it does what I expect -- it jumps to the search screen and searches for those terms. I see lots of results. If I then select "should" in the search box and replace it with "screen", it searches for "screen" and I see just a few results. The URL on the page updates. So far so good.

When I click the browser back button, the URL in the url bar changes but the previous results of the search aren't loaded. I'm still looking at the search results for "screen" even though the url says http://localhost:7070/doc/?pattern=screen. If I repeatedly press the forward and back buttons the URL will change but the page contents do not.

I tested this on Chrome, Safari and Firefox running on OSX.

yanne commented 10 years ago

True, it's good to have testers aboard :smile: I'll fix this. This would probably need a proper test too, because the functionality is no longer trivial.

The test infrastructure is something to consider also. I think we should do at least some e2e tests with Selenium, but there might also be need to write some more unit-test like tests for the javascript code. That would require using something like mocha + phantoms, which in turn would be dragging nodejs as a dev dependency. Do you have any thoughts on that?

yanne commented 10 years ago

I think I fixed back/forward between search terms.

yanne commented 10 years ago

I also functionality to render the library/resource file listing when search term is empty.

boakley commented 10 years ago

It still doesn't seem right. Yes, we should write some end-to-end tests. The irony of developing test tools without writing tests is very obvious to me. I will try to write some acceptance tests.

The problem is that going back prior to searching doesn't work.

yanne commented 10 years ago

Well, live and learn. One more try, now also handling the initial state object (which is, of course, null)

boakley commented 10 years ago

The back button is working, but you've broken navigation to specific keywords.

With the latest version, if I go to http://localhost:7070/doc/keywords/Selenium2Library/Add%20Cookie/ the "Add Cookie" keyword is not scrolled into view.

yanne commented 10 years ago

True, that was a functionality that I did not notice the original had. This is a victim of not embedding the js code directly into the template. I'll figure out a fix.

yanne commented 10 years ago

Okay, now the scrolling works as in original

boakley commented 10 years ago

Thanks, Yanne!