Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[a11y] Come up with plan to add a11y to our development process #5909

Closed
3 tasks
mcomella opened this issue Oct 9, 2019 · 8 comments
Closed
3 tasks

[a11y] Come up with plan to add a11y to our development process #5909

mcomella opened this issue Oct 9, 2019 · 8 comments
Assignees
Labels
access Accessibility: Talkback, HW keyboard/mouse, braile display etc. E5 Estimation Point: about 5 days eng:health Improve code health P2 Upcoming release

Comments

@mcomella
Copy link
Contributor

mcomella commented Oct 9, 2019

What

We already have a backlog of a11y issues. So what we want here would be: come up with a process for regularly testing and prioritizing these.

There are a lot of ideas in the comments here, so take those into account!

Acceptance Criteria

  • Reach out to Jamie Teh
  • What is the scope we want to add into Fenix process? Timebox to the labeled size.
  • File an issue explaining this plan

As part of this plan, having something that release management could look at for "release blocker/critical a11y bugs" would be helpful, to give sign-off on whether a release can go out.

also, @kglazko might be interested in a11y work, so loop her in/see if she's interested!


On FFTV, we neglected to check a11y for a while and now there are a lot of issues to fix. To avoid this rubber-banding on Fenix, we should ensure we're regularly verifying a11y behavior.

Here are some possible solutions (that can be combined), assuming we've already audited and fixed the a11y throughout the app (is that #4503?):

  • QA validates every new feature for a11y, e.g. as a test suite item
  • Devs validate every new feature for a11y, e.g. as a PR checklist item
  • Use automated a11y testing (like accessibility scanner)

@eeejay Do you have suggested best practices to regularly test a11y behavior so we can avoid regressions?

CC @liuche, @boek

┆Issue is synchronized with this Jira Task

@boek boek added access Accessibility: Talkback, HW keyboard/mouse, braile display etc. eng:health Improve code health labels Oct 15, 2019
@boek boek added this to Prioritized Eng Backlog in Fenix Sprint Kanban Oct 15, 2019
@liuche
Copy link
Contributor

liuche commented Oct 15, 2019

The action item here looks like: come up with a process for regularly testing and prioritizing these

(We already have a backlog of these items.)

A few quick immediate action times:

  • We can add a11y to the linter
  • Ask QA to file a11y issues

@MReschenberg do you have any suggestions other teams do this?

@MReschenberg
Copy link

MReschenberg commented Oct 15, 2019

Strong upvote on adding a11y validation to your PR process (CI?) so the responsibility falls on developers, re: @mcomella's suggestion above.

I'm (regrettably) not as knowledgeable about the dev process on fftv/fenix, but on the whole I don't think a11y should be a QA job. Features should be developed with a11y in mind, and developers become more mindful if the accessibility of their features part of the feature requirements.

Jumping to this model sounds like it'll be a bit painstaking, though, so here's some thoughts in the meantime:

  • QA should file a11y issues on existing, published features, and these issues should be prioritized
  • Before accepting/merging new PR's, do an a11y review (including sifting through the backlogged QA bugs to fix things relevant to the current feature, and ensure no /new/ a11y bugs would be created)
    • this is something we (try to) do on desktop, but it can be a mush-y process especially when no one on the team feels qualified to do this review; often ends up the responsibility of a11y team even for features we don't directly deal in.
    • If no one feels qualified to review, on desktop we add a flag on bugzilla; prioritization of those is at discretion of a11y team. We're small (but mighty!) and happy to offer advice, but we're spread pretty thin so we highly encourage other teams to learn how their work intersects with a11y.
  • add a11y linting, CI testing -- this is something desktop has :)

@sblatz sblatz added needs:group-triage P2 Upcoming release labels Oct 29, 2019
@boek boek changed the title [a11y] Ensure we regularly test a11y behavior [a11y] Add a11y to our development process Oct 29, 2019
@asadotzler
Copy link

First, I second MReschenberg's comment.

Second, we should endeavor to move accessibility as far left in the process as possible. Starting with our user research and design and then into engineering and code review, and at the end with QA we should only be catching edge cases and the like. A good starting place for engineering is to consider the content of this document https://docs.google.com/document/d/1NaCEIshLuXzqxJbpSNFmzeUigcMpReGD1JzPknnpvlo/edit#heading=h.otzjplb9vdv1 during development. It's not comprehensive but it covers a lot of important ground.

Third, when in doubt, reach out. I'm available for product-y advice and the a11y engineering team for code-y consultation.

@jcsteh
Copy link

jcsteh commented Oct 30, 2019

Further to the last two comments, while the ideal situation is that we don't rely on QA as the catch-point for a11y issues, I still think having QA validate new features for a11y is a good idea. This is especially true while you're still ramping up in other areas of a11y consideration; e.g. incorporating a11y into process, having UX do a11y annotations, ensuring engineers consider (and sign off on) a11y requirements, etc.

@liuche liuche changed the title [a11y] Add a11y to our development process [a11y] Come up with plan to add a11y to our development process Nov 8, 2019
@liuche liuche added the E5 Estimation Point: about 5 days label Nov 8, 2019
@liuche liuche moved this from Prioritized Eng Backlog to Sprint Backlog in Fenix Sprint Kanban Nov 8, 2019
@mcarare
Copy link
Contributor

mcarare commented Nov 19, 2019

I think we can gradually add accessibility checks in our UI tests. Either just checking that views also have a certain content description (or a non-empty one) or implement something similar to that suggested here:
https://tech.ebayinc.com/engineering/android-accessibility-automation-with-espresso/

@rpappalax
Copy link
Contributor

Further to the last two comments, while the ideal situation is that we don't rely on QA as the catch-point for a11y issues, I still think having QA validate new features for a11y is a good idea. This is especially true while you're still ramping up in other areas of a11y consideration; e.g. incorporating a11y into process, having UX do a11y annotations, ensuring engineers consider (and sign off on) a11y requirements, etc.

I agree with above comments. As we develop features, we can add more manual (and UI automated) testcases to verify those features work as intended. FYI, SV has created some testcases around accessibility already: https://testrail.stage.mozaws.net/index.php?/search/index/59

@sblatz sblatz self-assigned this Dec 10, 2019
@liuche
Copy link
Contributor

liuche commented Dec 12, 2019

I've been talking to Ritu from relman, and one thing that she'd like to check is "are there any critical a11y bugs that would block release". Ideally, another outcome of this is to also have a github query (or something, need not be github) that she can look at for a release signoff.

@sblatz sblatz moved this from Sprint Backlog to In Progress in Fenix Sprint Kanban Dec 17, 2019
@sblatz sblatz closed this as completed Jan 15, 2020
@project-bot project-bot bot moved this from In Progress to Sprint 20.1 DONE in Fenix Sprint Kanban Jan 15, 2020
@sblatz
Copy link
Contributor

sblatz commented Jan 15, 2020

Updates I communicated out to the fenix team:

During design reviews, we should ask ourselves if custom UI is necessary to accomplish the desired behavior. Oftentimes, out of the box android UI has accessibility and localization built in, whereas with custom pieces we need to build it ourselves.

For new features that have UI elements that are unique to Fenix or have user interaction, test them personally with TalkBack (just as you would test to make sure the feature works before submitting a patch)

Before submitting a patch, run the Accessibility Scanner over the screens affected to ensure no new issues are introduced (and post a screenshot of your results)

QA should test tickets that have “eng:qa:needed” with accessibility services (like TalkBack) enabled and only mark them as “eng:qa:verified” if these services work well with the new feature

As mentioned in the ticket, QA team will be adding accessibility checks to UI tests over time

Remember: the earlier we catch issues in the process (closer to dev coding or UX designing) the less work it is for everyone involved! So please be diligent about going through these checks before submitting a patch.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
access Accessibility: Talkback, HW keyboard/mouse, braile display etc. E5 Estimation Point: about 5 days eng:health Improve code health P2 Upcoming release
Projects
None yet
Development

No branches or pull requests

9 participants