Unexpected behavior note follow up #596
Conversation
client/components/TestRun/index.jsx
Outdated
| unexpectedBehaviors.push({ id: 'AT_CRASHED' }); | ||
| if (i === 4) | ||
| unexpectedBehaviors.push({ id: 'BROWSER_CRASHED' }); | ||
| if (i === 0) { |
There was a problem hiding this comment.
REVIEWERS: this is the main concern here. Why does graphql expect a string while the TestRenderer expect an object with an id?
| ) | ||
| return testResult; | ||
| ) { | ||
| // Mapping unexpected behaviors to expected TestRenderer downstream format |
There was a problem hiding this comment.
REVIEWERS: I had to do this so that the TestRenderer would receive consistent data.
client/resources/aria-at-harness.mjs
Outdated
| ) | ||
| ), | ||
| fragment( | ||
| // TODO: Figure out why this isn't appearing |
There was a problem hiding this comment.
Did I write this lol? Can we remove this comment or is it still relevant?
There was a problem hiding this comment.
Thank you for reviving this ancient PR, thinking of the conflicts you probably had to fix makes me sweat! I did notice one issue when testing it, which is the case where I select that "Yes, there were unexpected behaviors" and then fail to specify any of the radio buttons. When I hit submit, the toggle switches from "Yes" to "No", but more importantly the error is not focused or scrolled to, forcing me to look over all the forms trying to find the one that didn't work.
| ? scenarioResult.unexpectedBehaviors.map( | ||
| behavior => behavior.id | ||
| ) | ||
| : scenarioResult.unexpectedBehaviors |
There was a problem hiding this comment.
Makes sense! This could be simplified slightly to scenarioResult.unexpectedBehaviors ?? scenarioResult.unexpectedBehaviors.map(behavior => behavior.id)
|
One important note is that the unexpected behavior note PR for the aria-at repo will also need to be updated. I wouldn't expect as many merge conflicts, which is good. It's important to make sure the test runner is the same to prevent any very confusing issues from popping up. |
The error is scrolled to and |
|
@alflennik You should now be able to observe that when you select "Yes" with no unexpected behaviors and submit the form, the page scrolls to the required element and "Yes" is not toggled to "No". |
This PR encompasses @alflennik's commits with a merge to main and a few other changes. w3c/aria-at#802 must also be merged alongside this change. When this is merged, #450 can be deleted.
Steps to test: