richie icon indicating copy to clipboard operation
richie copied to clipboard

Improve accessibility of the sale tunnel

Open manuhabitela opened this issue 2 years ago • 2 comments

Purpose

Make sure keyboard and screen reader users can navigate through a course product widget and its sale tunnel.

Proposal

There are a few tricky stuff to get right here:

  • make sure things announced by screen readers are in correct order
  • make sure things announced are explicit enough without having to navigate around items to understand the context
  • make sure navigation between steps is handled correctly
  • make sure navigation between payment info/address creation is handled correctly
  • make sure the address form is accessible

Opening this early for the curious @jbpenrath @sandroscosta.

I'm currently on step 2, on the address form. Didn't get a chance to check the last step, and the product widget after the user bought it. Also didn't update the tests for now.

manuhabitela avatar May 11 '22 17:05 manuhabitela

As a heads-up, the address form will need some visual changes.

  • for now, the submit button is disabled until the form is valid. I'd advice to change this behavior that is usually extremely confusing for screen reader users (as the button is not announced), but also for sighted users. So that people can submit the form and have feedback if there are errors. This article sums it up well: https://axesslab.com/disabled-buttons-suck/
  • another thing is that the asterisk need to be explained in plain text, preferably before the form inputs
  • errors will need to be explained. For now if I submit the form while it's invalid, no errors are shown, only red borders.

manuhabitela avatar May 11 '22 17:05 manuhabitela

Well, this took more work than I thought!

This PR is getting quite big but all code is split in small, descriptive commits, so hopefully that shouldn't be difficult to review. If easier we could review it together via jitsi.

Remaining stuff to do is:

  • [x] update the tests
  • [x] see how we handle all the field error messages (see video 3)? I guess we could handle them more globally than on the address form only.

Here are a few videos of notable changes. Most changes are made so that keyboard/screen reader users don't get lost, but a few changes are on the actual UI, like the form submit buttons that are now clickable directly (to get feedback on errors).

https://user-images.githubusercontent.com/221253/171220651-32505b27-589a-41cb-8e5f-8d17bc7d57bd.mp4

https://user-images.githubusercontent.com/221253/171220646-b55c269a-ae02-4a6d-bee6-5efa9781a591.mp4

https://user-images.githubusercontent.com/221253/171220638-ff99d177-efb9-4525-99c9-c1c3d37b375e.mp4

manuhabitela avatar May 31 '22 16:05 manuhabitela

I finally added all the tests, from my side this is good to merge :pray:

Added the tests in the corresponding commits.

manuhabitela avatar Aug 17 '22 16:08 manuhabitela

Thanks for reviewing, updated after your feedback. As with my other PR I don't know why the cookicutter-bootstrap test fails. Maybe because this test didn't exist when the PR was created?

I also don't get why a backend test doesn't pass as I don't think I changed anything related to it.

manuhabitela avatar Sep 22 '22 12:09 manuhabitela

Oh the bootstrap-cookiecutter job fails because your branch exists on a fork not on the repository...

About the backend test failure, it's maybe a flaky test 🤔

=================================== FAILURES ===================================
_______________ PersonFactoryTestCase.test_factories_person_bio ________________

self = <tests.apps.courses.test_factories_person.PersonFactoryTestCase testMethod=test_factories_person_bio>

    def test_factories_person_bio(self):
        """
        PersonFactory should be able to generate plugins with a realistic bio for
        several languages.
        """
        person = PersonFactory(page_languages=["fr", "en"], fill_bio=True)
    
        # Check that the bio plugins were created as expected
        bio = person.extended_object.placeholders.get(slot="bio")
        self.assertEqual(bio.cmsplugin_set.count(), 2)
    
        # The bio plugins should contain paragraphs
        for language in ["fr", "en"]:
            bio_plugin = bio.cmsplugin_set.get(
                plugin_type="PlainTextPlugin", language=language
            )
>           self.assertTrue(bio_plugin.plain_text_plaintext.body.count(".") >= 2)
E           AssertionError: False is not true

tests/apps/courses/test_factories_person.py:55: AssertionError
=========================== short test summary info ============================
FAILED tests/apps/courses/test_factories_person.py::PersonFactoryTestCase::test_factories_person_bio
================== 1 failed, 1110 passed in 596.13s (0:09:56) ==================
2022/09/22 12:42:18 Command exited with error: exit status 1

Exited with code exit status 1
CircleCI received exit code 1

jbpenrath avatar Sep 22 '22 17:09 jbpenrath

We did it :sweat_smile: thanks

manuhabitela avatar Sep 23 '22 12:09 manuhabitela