search

commonsearch / cosr-front

Frontend of Common Search. Go server for fetching and rendering results + HTML5 UI to browse them.
https://uidemo.commonsearch.org
Apache License 2.0
61 stars 13 forks source link

Keyboard navigation #10

Open sylvinus opened 8 years ago

sylvinus commented 8 years ago

We should make sure the interface is completely usable from the keyboard only.

Do we need to use tabindex attributes?

JBaba commented 8 years ago

I am interested and i want to contribute anyway i can but i have never done this before.

sylvinus commented 8 years ago

Great @JBaba JBaba! We're here to help :)

I think a good first step could be to look at this feature on other search engines, and try to make a spec of what we want to implement:

JBaba commented 8 years ago

Appreciate your quick response. I will analyze other search engines features and make a list that might be missing from Commonsearch. I hope I can test on this https://uidemo.commonsearch.org url to find out missing features.

JBaba commented 8 years ago

By default all keyboard shortcuts are enabled and tab-index also works fine.

Key/features we need :

  1. If focus is on any links then by pressing up and down arrow keys we can navigate to links accordingly.
  2. By pressing "Esc" key focus goes to HTML document after this by pressing up and down arrow key page can be scrolled accordingly.
sylvinus commented 8 years ago

That seems reasonable for a first version! The page should indeed have 2 different states depending on where the focus is.

JBaba commented 8 years ago

Only this features needed at this point rest are needed based on information displayed on different search engines web pages. Let me know what is next step for me.

sylvinus commented 8 years ago

Great! We wrote a tutorial which you can follow here: https://about.commonsearch.org/developer/tutorials/first-frontend-patch

JBaba commented 8 years ago

@sylvinus I have successfully implemented case 2.

Need your guidance on case 1 this is what i have in mind, Problem: Case 1 is caused by "tabindex" attribute which we don't have in document elements.

Observation: If end user has windows/linux operating system then browsers on system supports focus in and out somehow without "tabindex" not sure how!! then add our functionality.

Solution: Do we want to dynamically assign "tabindex" attribute in index.js file ? or we assign "tabindex" attribute to index.html elements then dynamically assignments for search results ?

sylvinus commented 8 years ago

I think it would be better for accessibility and text-based browsers to have some minimal tabindex attributes in the HTML.

There indeed seems to be some inconsistencies within browsers & OSes with focus. Let's try to have it work as intended in one browser first and we'll expand to others afterwards?

JBaba commented 8 years ago

I have raised pull request for this issue and both cases has been addressed in this patch https://github.com/commonsearch/cosr-front/pull/31

sylvinus commented 8 years ago

@JBaba okay now we have UI tests running on all browsers! https://github.com/commonsearch/cosr-front/blob/master/tests/webdriver/keyboard.js https://saucelabs.com/open_sauce/user/commonsearch

JBaba commented 8 years ago

@sylvinus Help me understand this. You want me to complete this selenium test cases. And after that test at this provided URL ?

sylvinus commented 8 years ago

You can run them locally with make docker_uitest. You mentioned differences between browsers so the tests are a good way to make sure we agree on what the behaviour should be for each action!

JBaba commented 8 years ago

@sylvinus Hey I am trying to write test case but having hard time to find command guide. even online and github examples are missing this commands

 yield browser.keys(["TAB","TAB","TAB","TAB","TAB"]);
 var focused = yield inspectElement(yield browser.elementActive());
sylvinus commented 8 years ago

@JBaba they should be documented on http://webdriver.io

JBaba commented 8 years ago

@sylvinus 

yield browser.keys(["\ue015","\ue015","\ue015","\ue013","\ue015"]);
inspectValues = yield inspectElementName(yield browser.elementActive());
assert.equal(inspectValues.tagName, "a");

// Arrow down should select the next result.
yield browser.keys(["\ue013","\ue015"]);

First keys sequence works perfect and after that result for second keys sequence is, 

'body' == 'a'
running phantomjs
AssertionError: 'body' == 'a'
    at Context.<anonymous> (tests/webdriver/keyboard.js:57:16)

Focus goes back to <Body> tag instead of going next <a> tag. You can find test case in my forked branch.

sylvinus commented 8 years ago

Great, I will have a look! Very happy that we have tests like these so we can all agree on the behaviour in the test and then fix the code :)