Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge with master #13

Merged
merged 15 commits into from
May 15, 2020
Merged

merge with master #13

merged 15 commits into from
May 15, 2020

Conversation

AhmedAbdoOrtiga
Copy link
Owner

Description of changes

Pull request checklist

  • Addresses an existing issue: #0000
  • Ran yarn fastpass
  • Added/updated relevant unit test(s) (and ran yarn test)
  • Verified code coverage for the changes made. Check coverage report at: <rootDir>/test-results/unit/coverage
  • PR title AND final merge commit title both start with a semantic tag (fix:, chore:, feat(feature-name):, refactor:). See CONTRIBUTING.md.
  • (UI changes only) Added screenshots/GIFs to description above
  • (UI changes only) Verified usability with NVDA/JAWS

waabid and others added 15 commits May 14, 2020 14:16
#### Description of changes

#2521 tracks an issue where it wasn't clear what the expectations on a user were for testing the Landmarks "primary content" and "no repeating content" requirements on pages with no landmarks.

The proposed fix for that was a text update + making the requirements pass automatically on pages that have no landmarks.

This fix involves a significant bit of new behavior. We wanted to *keep* the 2 requirements in question as manual requirements, not assisted requirements, since it's possible for a page to have no main landmarks and for a user to need to manually enter a failure not associated with any instance we might show in an instance list. However, this is the first time we're assigning auto-pass behavior to a manual test; in all other cases where we support auto-pass, it works by looking for an assisted requirement with no matching instances.

This new behavior is especially challenging because the set of instances required for auto-passing ("are there **any** landmarks on the page?") is different from the set of instances required for visualization ("what are all the **main** landmarks on the page?").

To support this, I've added 2 new features to `AssessmentStore`'s `Requirement` processing:
* A new `isVisualizationSupportedForResult` property which `Requirement`s can use to indicate that certain results returned by their analyzers should not support being visualized (by default, all results support visualization)
* A new `getInitialManualTestStatus` property which manual `Requirement`s can use to infer an initial pass/fail state after scanning for results (by default, a manual requirement is put in the UNKNOWN state regardless of analysis results)

These are used by the updated requirements by updating their analyzers to use the `unique-landmark` rule to look for *all* landmarks instead of the old behavior of looking only for main landmarks, use `isVisualizationSupportedForResult` to only visualize the main landmarks (like before), and use `getInitialManualTestStatus` to set a "PASS" status if the scan completes and there are no instances with the landmark role data from the `unique-landmark` rule.

New behavior:

**no landmarks: "Primary content" and "No repeating content" automatically pass**
![screen recording of no-landmarks interactions](https://user-images.githubusercontent.com/376284/81753659-1ef3ea00-9469-11ea-8646-e394ffff61de.gif)

**only non-main landmarks: "Primary content" and "No repeating content" don't auto-pass and show no matching instances to visualize**
![screen recording of banner-landmark-only interactions](https://user-images.githubusercontent.com/376284/81753648-1ac7cc80-9469-11ea-9c90-05f8d7243088.gif)

**Mix of main and non-main landmarks: "Primary content" and "No repeating content" don't auto-pass and visualize only the main landmark**
![screen recording of mixed-landmarks interactions](https://user-images.githubusercontent.com/376284/81753619-0a175680-9469-11ea-9d6e-40131889cf46.gif)


#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox -->
- [x] Addresses an existing issue: #2521
- [x] Ran `yarn fastpass`
- [x] Added/updated relevant unit test(s) (and ran `yarn test`)
- [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage`
- [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`.
- [x] (UI changes only) Added screenshots/GIFs to description above
- [x] (UI changes only) Verified usability with NVDA/JAWS
* add rule link

* update test name

* fix a url

* share base url
…ly (#2684)

* feat(nav-bar-content): nav link handler uses requirement key explicitly

* re-added removed line
…onent (#2686)

* using optional link component

* uts

* update all guidance link usage

* fix report-html-generator deps

* revert a change

* update snapshots
#### Description of changes

Adds new end to end tests that verify the auto-pass-on-no-landmarks behavior introduced in #2644. Updates the product code to add a new data-automation-id, no UI impact.

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox -->
- [x] Addresses an existing issue: e2e test for behavior in #2521
- [x] Ran `yarn fastpass`
- [x] Added/updated relevant unit test(s) (and ran `yarn test`)
- [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage`
- [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`.
- [n/a] (UI changes only) Added screenshots/GIFs to description above
- [n/a] (UI changes only) Verified usability with NVDA/JAWS
@AhmedAbdoOrtiga AhmedAbdoOrtiga merged commit 64dc031 into AhmedAbdoOrtiga:master May 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants