Space key not handled in close button

[mlewand]

1. Open Accessibility Checker manual test.
2. Open AC by pressing it's icon.
3. Press shift + tab twice to move the focus on close button.
4. Press space.

Expected

AC gets closed.

Actual

Key event does not get handled, and default browser behavior apply (scrolling the page).

[mlewand]

@f1ames could you please review this one?

[f1ames]

@Tade0 There are still unanswered comments from @mlewand, I also added one myself, please take a look there - #206.

[Tade0]

A curious thing occurred to me: the listeners that I've been manipulating seem not to be the ones that actually do the job. Investigating now what's happening here.

[Tade0]

There's an issue with this issue, take a look at these two branches:
https://github.com/cksource/ckeditor-plugin-a11ychecker/tree/t/200-redundant
https://github.com/cksource/ckeditor-plugin-a11ychecker/tree/t/200-broken

If you comment out the listeners in ui/Viewer, you can still use Return/ESC/click to close AC.
If you, in turn, comment the listener in ui/ViewerController the Return/click behaviour is changed and although it closes the panel, it doesn't really close AC.

This could be easily fixed with breaking ecapsulation/separation of concerns by making the Viewer aware of the Controller, but I think this is not the way to go here.

My proposition is to implement an "end"(as in: "close AC") event on the Viewer and listen on it in the ViewerController the events in Viewer would fire that event and the ViewerController would act accordingly.

What do you think @mlewand , @f1ames ?

[f1ames]

It looks like a redundant code indeed. But I am not sure if there wasn't a reason for that. I would check to be sure if hiding the balloon panel (that's what happens in the Viewer: // Hide the panel...) is same as closing the whole a11ychecker (the comment in ViewerController seems to confirm that it is the same, duplicated behavior).

Apart from that, I think the approach with emitting close/end event inView and listening to it in ViewController is a way to go.

[mlewand]

@Tade0 I've reviewed #215 solution, find my notes in PR ticket.

[f1ames]

@Tade0 I added a comment to your PR ticket.