adaptlearning / adapt-contrib-narrative

A component that displays an image gallery with accompanying text
GNU General Public License v3.0
5 stars 39 forks source link

Tabbing fixes for ABU-1142 #156

Closed chris-steele closed 8 years ago

chris-steele commented 8 years ago

This should resolve the issue described in ABU-1142. From my testing there seemed to be multiple related issues, but these should be resolved with this PR. One thing to note, when a narrative is replaced with its original hot graphic component (when resizing the browser window from mobile size back to desktop size), the component-inner div carries the aria-label and tabindex attributes because a11y_update hasn't run. This means that when the hot graphic popup is opened the inner div ends up in the tabbing order. When the popup is closed a11y_update is run and this issue appears no more. Obviously just sticking in a call to a11y_update when the narrative is replaced by the hotgraphic will fix this, but can someone advise if this is the right way to go? Thanks.

moloko commented 8 years ago

One thing to note, when a narrative is replaced with its original hot graphic component (when resizing the browser window from mobile size back to desktop size), the component-inner div carries the aria-label and tabindex attributes because a11y_update hasn't run. This means that when the hot graphic popup is opened the inner div ends up in the tabbing order.

Is this what's causing it to send focus to the instruction text of the narrative component c-30 when you resize down to mobile size?

chris-steele commented 8 years ago

The focus jumping to c-30 instruction was because redundant resize events were getting fired, which meant replaceInstructions was being called - so I added a guard to avoid calling this if the screen size hasn't actually changed.

moloko commented 8 years ago

sure, but it's still switching focus to the instruction text of a completely different component when it switches to narrative

chris-steele commented 8 years ago

I'm not able to replicate that - when I shrink the window down to mobile size focus just gets pushed to the top of the page.

moloko commented 8 years ago

gah, I've just retested and it's not happening now...!

I see what you mean about 'inner div ends up in the tabbing order'; your proposed solution sounds OK to me...

chris-steele commented 8 years ago

Don't worry I've had some odd goings-on. For some reason in Firefox I can't get the a11y toggle button to show up. Yesterday it was fine, so something must have happened overnight I can only assume. Weird!

moloko commented 8 years ago

sticking in a call to a11y_update when the narrative is replaced by the hotgraphic will fix this

Are you still planning to add this in @chris-steele ?

chris-steele commented 8 years ago

Sure, I'll sort that out ASAP.

moloko commented 8 years ago

+1

brian-learningpool commented 8 years ago

+1

oliverfoster commented 8 years ago

+1