bbcmicrobit / PythonEditor

A MicroPython editor for the BBC micro:bit that works with browsers.
https://python.microbit.org/
MIT License
198 stars 130 forks source link

A11y accessible modals #290

Closed microbit-mark closed 5 years ago

microbit-mark commented 5 years ago

fixes https://github.com/microbit-foundation/platform-software-issue-tracker/issues/407 and fixes https://github.com/microbit-foundation/platform-software-issue-tracker/issues/519 does not include error modals

microbit-carlos commented 5 years ago

Clicking inside the modals causes this "selected" effect:

image

image

Can this be avoided?

microbit-carlos commented 5 years ago

Fixed the issue with the table cells being selectable in this PR: https://github.com/microbit-mark/PythonEditor/pull/1

microbit-mark commented 5 years ago

We can use blur on that element. Have done so inline https://github.com/bbcmicrobit/PythonEditor/pull/290/commits/02b67c66fdf5d0fa2b698c0d9a27f5a34f5398d8, but not sure if that's the best approach.

microbit-carlos commented 5 years ago

Might be better to just remove the focus CSS? Btw, this branch has a couple of conflicts with master, better resolve them now while we are testing, so that we don't have to retest later.

microbit-mark commented 5 years ago

Have moved this to css and fixed the conflict

microbit-carlos commented 5 years ago

Snippets are not working on Safari, were they working before?

image image

microbit-carlos commented 5 years ago

To avoid having something always focused when using the mouse, could we do this?

Alternatively, if we focus the element that we just edited to remove the focus CSS (https://github.com/bbcmicrobit/PythonEditor/pull/290/commits/01eb6dc220444f284f7d663b4df21fb2e849dd31) that would still have an element inside the modal focused and let users tab through the modal.

2019-10-31 13 09 49

microbit-mark commented 5 years ago

Snippets are not working on Safari, were they working before?

I can't recreate in Safari 13.0.2. But Safari 12 exhibits the issue.

To avoid having something always focused when using the mouse...

I think good practice would be to pass the focus to the modal and trap it and show element focus if the user has entered the modal via keyboard. If they have used the mouse, don't show focus.

I have added a commit that focusses the div rather than the first selectable element

microbit-mark commented 5 years ago

@microbit-carlos syntax error I think https://github.com/bbcmicrobit/PythonEditor/pull/290/commits/8367008d8dd47fb4ee86f8a39fe71ee58c4b6415 only Safari prior to 13 complained about it. :)

I've tried this in browserstack, can you test locally when you have a moment @microbit-carlos

microbit-carlos commented 5 years ago

It works! Looks like i accidentally deleted one bracket more than necessary, thanks Mark!

microbit-mark commented 5 years ago

Seem to have introduced another issue somewhere with the files help, now i've made it a button element

microbit-carlos commented 5 years ago

@microbit-mark is this PR ready or are there still things to iron out?

microbit-mark commented 5 years ago

Ready :)

microbit-carlos commented 5 years ago

This is shaping up fantastically, great work Mark! What browsers has this been tested in its latest iteration? Taking in consideration how difficult it was to get it working in Chrome and Safari at the same time I think there is a high risk of having issues in other browsers. So based on previous experiences I think we should test it in:

The testing should include opening the modals, iterating through the elements, activating a few of them with the keyboard, click on a couple of elements to ensure the click handlers are still working, and test at least one of the snippets with the keyboard and the mouse.

microbit-mark commented 5 years ago

Tests

✔️ Safari 13 Mac Mojave ✔️ Safari 12 Mac Mojave - Doesn't trap focus, can't focus on elements ✔️ ie10 Win 7 ✔️ ie11 Win7 ✔️ Edge 17 Win 10 ✔️ FF 70 Win 10 ✔️ FF 70 Mojave can't tab to open modals, all other checks pass ✔️ Chrome 78 Mojave ✔️ Chrome 78 Win 10

@microbit-carlos This works in all browsers, but for mac, full keyboard accessibility needs to be enabled and tabbing in safari

microbit-mark commented 5 years ago

Have also added https://github.com/microbit-foundation/python-editor/pull/107 as per https://github.com/bbcmicrobit/PythonEditor/pull/290#discussion_r342159385