Keybord nevigation Issue#10

[JBaba]

Two cases are addressed in this patch which is raised in Issue#10,

1. Case No 1 to provide traversing of search link based on Up and Down key press event I have added "tabIndex" attribute in html page and index.js where search links are being rendered. Two functions in Index.js has been added which are "traverseElementOnKeyDown(event)" and "findElementByAttributeValue(e,v)".
2. Case No 2 by pressing "Esc" key send focus back to html page also has been addressed.

[sylvinus]

Thanks @JBaba for the PR! Looks very good. I've added a couple comments!

[sylvinus]

This works great!

A couple things I think we could improve:

• Add a better style for active links. Maybe just text-decoration:underline?
• When using the arrow keys, the page scrolls a bit each time. This doesn't happen with the "tab" key. I think we should avoid this behaviour (a return false or event.preventDefault() may do the trick?)
• When you arrive at the end of the list, the next "tab" keypress goes back on top, to the first <div class="u"> element. Maybe instead of giving a specific tabindex to each h3.a, we could just explicitly give tabindex=-1 to the <div class="u"> elements?

[JBaba]

I was about to address some of this that's great that you just mentioned. I didn't know how could I address this changes while I was in middle of development. I will work on this addressed changes. On Mar 17, 2016 9:19 PM, "Sylvain Zimmer" notifications@github.com wrote:

This works great!

A couple things I think we couple improve:

• Add a better style for active links. Maybe just text-decoration:underline?
• When using the arrow keys, the page scrolls a bit each time. This doesn't happen with the "tab" key. I think we should avoid this behaviour (a return false or event.preventDefault() may do the trick?)
• When you arrive at the end of the list, the next "tab" keypress goes back on top, to the first
  element. Maybe instead of giving a specific tabindex to each h3.a, we could just explicitly give tabindex=-1 to the

  elements?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/commonsearch/cosr-front/pull/31#issuecomment-198151745

[JBaba]

@sylvinus Can you help me understand this "next" and "previous" behaviour because after clicking on either of this "renderHit" function is not being invoked and which results into tabIndex not being assigned to search links. I don't know what's happening here ?

[sylvinus]

Currently the "next" links are regular links that cause a page reload. So the page you get has been rendered from the Go template, not from JavaScript's renderHit()

[JBaba]

So only first page search result will use JavaScript's renderHit(). What's solution for rest of the pages and how can I help ? Even if you come back from second page to first by clicking on previous then also we don't have tabIndex

[sylvinus]

@JBaba it's the other way around: all results except the first page load use renderHit().

You have already patched index.html to add some tabindexes, you just need to make the same changes there are you make in renderHit!

[JBaba]

I have tried to use $index but it always gives me 0,1,2,3.... series to start at my desired value what can I do. Let me know your views I have never used GoLang before.

[sylvinus]

Can't you do {{ add $index offset}} ?

[JBaba]

@sylvinus For this we have to define add function. I was reading similar article on this.

[sylvinus]

If you need to do that you can add it here: https://github.com/commonsearch/cosr-front/blob/master/server/templates.go#L83

[JBaba]

Thanks that worked.

[JBaba]

I have addressed new changes. Need help with scrolling page to top when we reach at end of screen but preventDefault() interrupts that. I have tried using this code,

var h = window.innerHeight;
var rect = currentElement.getBoundingClientRect();
var offset = h - rect.top;
if(offset < 100 ){
    window.scrollTo(0, h/2);
} 

But this works for sometimes and sometimes not. Couldn't figure out why ?

[sylvinus]

Scrolling to h/2 seems suspicious to me.. Shouldn't it be rect.top?

[JBaba]

This what i have found after referring few posts. I am finding offset (bottom) by subtracting document inner width from top of element. I was also not sure and after tested for while felt that this is buggy so asked for helped. What do you suggest how should we approach this ?

[sylvinus]

It seems like the general idea should work, but the formula doesn't look right. Try window.scrollTo(0, rect.top);?

[JBaba]

@sylvinus I have simulated problem here you can see what's happening, https://jsfiddle.net/naimish101/bat8bf8h/4/

[sylvinus]

@JBaba Seems to be fixed if you use offset < 0 instead ?

[JBaba]

@sylvinus you must be testing on chrome. It does not work vary well with other browsers.

[sylvinus]

It seems to work for me in Firefox as well.

I'm working on interface tests for #1 so I'm going to merge it now and with Saucelabs we will make sure it behaves the same way in all browsers!

Thanks for the great work!