search

boakley / robotframework-hub

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

search page is unstyled #29

Closed boakley closed 10 years ago

boakley commented 10 years ago

prior to the incremental search being added, the url http://localhost:7070/doc/keywords/?pattern=should could be used to see search results.

After merging the incremental-search feature, that page is now unstyled.

boakley commented 10 years ago

Yanne, I discovered this bug after the merge. Can you take a look at it?

Also, I noticed that when you update the URL bar, the url isn't canonical. The canonical url for a search should be "http://localhost:7070/doc/keywords/?pattern=search". However, you just append "?pattern=search" to whatever the current URL is.

For example, go to http://localhost:7070/doc/keywords/BuiltIn/Fail/. Then, type in "should" in the search box. The URL becomes "http://localhost:7070/doc/keywords/BuiltIn/Fail/?pattern=should" but it should be "http://localhost:7070/doc/keywords/?pattern=should.

I want to keep symmetry with the api. Each of this should get a list of keywords that match a pattern, one as json, one as a styled page:

Thanks!

yanne commented 10 years ago

Hmm.

This requires some more thought, and I am not really sure about the benefits of mirroring the API and UI in URL level. What are the downsides of having just one URL for browser, where the additional configuration could be done via query parameters. I actually had the idea of also showing the library documentation without doing a complete page load, but that would also logically mean getting rid of separate keywords/BuiltIn/ like URLs, instead opting for doc/?library=BuiltIn&target=Fail

If we want to have both the mirrored URLs and ability to do partial page loads like in the incremental search case, I'll need to do some more design. It's probably possible, though.

Meanwhile, if this is blocking further development, should we revert the branch merge?

boakley commented 10 years ago

This was my mistake for not documenting the URL scheme. My intention was for the /doc/keywords URL work the same way as /api/keywords (as documented here: RESTful API

I really like keeping the URLs identical between API and web page. It's a variation of what Jenkins does. The idea is, if you are on a web page and want the same information but via an API, all you do is replace "/doc" with "/api". That makes the API learning curve almost zero.

I don't want a single URL with parameters, I think that would make the URL structure harder to understand. Eventually the hub will serve test cases, test results, files on the file system, maybe other things. I want the URL structure to be of the form /doc/object-type/collection-name/item-name (eg: /doc/keywords/BuiltIn/Fail, /doc/tests/SmokeSuite/TestCase1, etc). If you use /doc/keywords you get all keywords, /doc/keywords/BuiltIn gives you only the keywords in the BuiltIn library, and /keywords/BuiltIn/Fail would give you that keyword.

At any stage you should be able to add ?pattern. For example, to show keywords that match "should" only in the BuiltIn library would be /doc/keywords/BuiltIn/?pattern=should. Likewise, to get the test suites matching "baz" in the suite foo inside suite bar it would be /tests/foo/bar?pattern=baz, and all test cases matching "baz" would be /tests/?pattern=baz

Does that make sense?

On Wed, Aug 13, 2014 at 11:47 AM, Janne Härkönen notifications@github.com wrote:

Hmm.

This requires some more thought, and I am not really sure about the benefits of mirroring the API and UI in URL level. What are the downsides of having just one URL for browser, where the additional configuration could be done via query parameters. I actually had the idea of also showing the library documentation without doing a complete page load, but that would also logically mean getting rid of separate keywords/BuiltIn/ like URLs, instead opting for doc/?library=BuiltIn&target=Fail

If we want to have both the mirrored URLs and ability to do partial page loads like in the incremental search case, I'll need to do some more design. It's probably possible, though.

Meanwhile, if this is blocking further development, should we revert the branch merge?

— Reply to this email directly or view it on GitHub https://github.com/boakley/robotframework-hub/issues/29#issuecomment-52075883 .

yanne commented 10 years ago

I see your point. In my opinion, there are a couple of issues with that scheme.

1) having everything under doc/ seems wrong, especially if you consider an API with capability of doing changes. For an editor, we'd need an API that just mirrors the filesystem. Nothing to do with documentation.

2) The behavior of search box within different pages is still unclear to me. Currently there are 3 possible scenarios i) We are in the root, doc/, in which case the search field searches keywords from all sources ii) We are in specific library or resource doc/keywords/LibraryName, in which case the search is restricted to the library or keyword iii) We have an URL like doc/keywords/LibraryName/KeywordName. This is unclear to me. Should the pattern apply to the library or everything here.

Or should be like you suggested in comment 1, so that search is always applied to all libraries and resources.

Anyway, I think I'll revert the merge commit, since the requirement of always updating the URL may mean going back to full page loads at all stages to keep the history consistent. I will try to get it working with partial page loads and history manipulation, but that will require some more design and testing.

boakley commented 10 years ago

On Fri, Aug 15, 2014 at 3:47 PM, Janne Härkönen notifications@github.com wrote:

I see your point. In my opinion, there are a couple of issues with that scheme.

1) having everything under doc/ seems wrong, especially if you consider an API with capability of doing changes. For an editor, we'd need an API that just mirrors the filesystem. Nothing to do with documentation.

/doc is for html formatted data. html-formatted versions of files will be under /doc, for example /doc/file/MySuite.robot. This would be for browsing syntax-highlighted versions of the files. If you want the data in json format you'll go to /api/file/... and the raw data will probably be /file (though I haven't thought very hard about that last bit)

2) The behavior of search box within different pages is still unclear to

me. Currently there are 3 possible scenarios i) We are in the root, doc/, in which case the search field searches keywords from all sources ii) We are in specific library or resource doc/keywords/LibraryName, in which case the search is restricted to the library or keyword iii) We have an URL like doc/keywords/LibraryName/KeywordName. This is unclear to me. Should the pattern apply to the library or everything here.

Or should be like you suggested in comment 1, so that search is always applied to all libraries and resources.

I think the search box should always be applied to all libraries. That being said, eventually I want to be able to enter something like "library:Selenium2Library screenshot" and that might redirect to /doc/keywords/Selenium2Library?pattern=screenshot" (issue #18)

Anyway, I think I'll revert the merge commit, since the requirement of always updating the URL may mean going back to full page loads at all stages to keep the history consistent. I will try to get it working with partial page loads and history manipulation, but that will require some more design and testing.

I'm fine with you backing out the merge if this is getting too complex. However, can you not just make the very first search do a page load of the doc/keywords?pattern=whatever, and after that it works the way it does now?

yanne commented 10 years ago

I got this mostly working without reverting anything, but I have one more question. Should the root URL for keyword search be doc/keywords rather that doc? The link from the dashboard goes to doc, but if we want to keep the canonical search URL as doc/keywords, I think that should be the landing URL as well. This is especially relevant since there's no way to actually navigate to doc/keywords currently.

yanne commented 10 years ago

@boakley can you test with the latest master, I think I got most of the remaining issues fixed, but there's still probably something I did not take into account.

boakley commented 10 years ago

Yes, the root URL for keyword search should be /doc/keywords.

Eventually there will be pages for other things. So, for example, a search for testcases might be /doc/testcases.

The landing URL is /doc, because that is the home page for the documentation. It just so happens that right there there is only keyword documentation, but eventually that page will be different. It will have links to keyword documentation, testcase documentation, etc.

On Tue, Aug 19, 2014 at 1:27 AM, Janne Härkönen notifications@github.com wrote:

I got this mostly working without reverting anything, but I have one more question. Should the root URL for keyword search be doc/keywords rather that doc? The link from the dashboard goes to doc, but if we want to keep the canonical search URL as doc/keywords, I think that should be the landing URL as well.

— Reply to this email directly or view it on GitHub https://github.com/boakley/robotframework-hub/issues/29#issuecomment-52595036 .

yanne commented 10 years ago

Okay, this should be fixed now.

boakley commented 10 years ago

Looking much better! We can probably release this version, though I won't do that at least until the weekend -- I have some other stuff I want to do first.

Thanks for all your work!

On Tue, Aug 19, 2014 at 6:16 AM, Janne Härkönen notifications@github.com wrote:

Okay, this should be fixed now.

— Reply to this email directly or view it on GitHub https://github.com/boakley/robotframework-hub/issues/29#issuecomment-52619439 .

yanne commented 10 years ago

Closing this. Further improvements can probably get their own issues