-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat: Enhanced conflict review #1195
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stalgiag super impressive! Especially given that there some points of this request are still in discussion. I've left some points of feedback and wondering if you have any thoughts on them.
But I'm open to gathering additional feedback through a push to the sandbox environment if that's okay with you -- given that this is so new and may require some more fine-tuning.
Conflicts {title} {versionString} | ARIA-AT | ||
</title> | ||
</Helmet> | ||
<h1>Conflicts for Test Plan Report</h1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<h1>Conflicts for Test Plan Report</h1> | |
<h1>Conflicts for Test Plan Report {title} {versionString}</h1> |
Maybe also the AT and Browser for further specificity? But we can see what others think here as well since it is already listed in the meta details below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went back and forth on this. At first, I had the title, version string, at, minimum or required at version, and browser all in the title but that felt too busy. Then I added the meta details section to free up some complexity in the title. This could be a good middle point though. Changes here 0d029ce
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading it again, feels like the "Test Plan Report" makes this too wordy, ha! But let's see what others think first
Thanks for the feedback @howard-e ! I believe that I have addressed each of your points. If you agree then we could move this to sandbox to get feedback from others. |
* Address sandbox deployment issue (#1190) * Changes to pass sandbox deployment issues * Remove explicit install of npm during deploy * fix: Only match with single alphanumeric keys in "Assign Testers" dropdown search, ignore Tab (#1196) * * undefined requiredAtVersionName renamed to exactAtVersionName * Re-use versionString instead of relying on testPlanVersion.updatedAt (noticed because it wasn't being passed to the created GH Issue) * Create distinction between each conflict * Create unique disclosures * Correct the conflicts to be unique across assertions * Support including conflict markdown from conflicts issue creation * Update snapshot --------- Co-authored-by: Stalgia Grigg <[email protected]>
Added the additional requested headers. Let me know what you think @howard-e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks for addressing this last bit of feedback and as always, exciting to see you continuing to provide these quality of life improvements! :)
I've left inline suggestions on a small editorial change. Please feel free to merge afterwards
client/components/TestQueue/Conflicts/AssertionConflictsTable.jsx
Outdated
Show resolved
Hide resolved
client/components/TestQueue/Conflicts/UnexpectedBehaviorsConflictsTable.jsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Howard Edwards <[email protected]>
…ictsTable.jsx Co-authored-by: Howard Edwards <[email protected]>
Co-authored-by: Howard Edwards <[email protected]>
Co-authored-by: Howard Edwards <[email protected]>
@howard-e I committed your comma removal editorial changes. Thank you! |
This PR adds a dedicated page for reviewing conflicts between test plan runs in a test plan report.
addresses #975
The design and layout is a first attempt and will likely need a few rounds of fine-tuning.
Screenshot of Conflicts page with disclosures closed
A screenshot of a new page titled 'Conflicts for Test Plan Report'. Below the title, a section with h2 titled "Introduction" says "This page displays conflicts identified in the current test plan report. Conflicts occur when different testers report different outcomes for the same test assertions or unexpected behaviors" . Another section titled 'Test Plan Report' provides details about the specific test plan, including the version 'Select Only Combobox Example V22.03.17,' the assistive technology used 'NVDA (2020.4 and above),' and the browser 'Firefox.' Further down, the 'Conflicts' section indicates that there are three conflicts in the test plan report. These conflicts are listed in collapsible disclosures with titles: 'Test 4: Navigate backwards to a collapsed select-only combobox in interaction mode.', 'Test 6: Read information about a collapsed select-only combobox in interaction mode.', 'Test 7: Open a collapsed select-only combobox in reading mode.'Expanded View of Specific Conflict
A screenshot shows an expanded conflict in the "Conflicts" section. The conflict is for "Test 4: Navigate backwards to a collapsed select-only combobox in interaction mode." The table compares assertions like the role, name, and state of the combobox, showing the results from two testers. One tester passed all assertions, while the other failed the first assertion but passed the rest. The first column is the assertion description, the second two columns are for each of the two testers. The column with different assertion results has strong text. Below the table, related GitHub issues are linked with details about the Issue title, whether it is currently open, and who opened it. Below that there are two buttons one to raise a new issue and one to open the run as a specific tester.