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

feat: Enhanced conflict review #1195

Merged
merged 33 commits into from
Sep 17, 2024
Merged

feat: Enhanced conflict review #1195

merged 33 commits into from
Sep 17, 2024

Conversation

stalgiag
Copy link
Contributor

@stalgiag stalgiag commented Aug 16, 2024

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.'

The screenshot described above

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.

The screenshot described above

@stalgiag stalgiag changed the title Enhanced conflict review feat: Enhanced conflict review Aug 19, 2024
Copy link
Contributor

@howard-e howard-e left a 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.

client/components/common/ReportStatusSummary/index.jsx Outdated Show resolved Hide resolved
client/components/common/ReportStatusSummary/index.jsx Outdated Show resolved Hide resolved
client/routes/index.js Show resolved Hide resolved
Conflicts {title} {versionString} | ARIA-AT
</title>
</Helmet>
<h1>Conflicts for Test Plan Report</h1>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<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.

Copy link
Contributor Author

@stalgiag stalgiag Aug 20, 2024

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

Copy link
Contributor

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

@stalgiag
Copy link
Contributor Author

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 <stalgia@bocoup.com>
@stalgiag
Copy link
Contributor Author

Added the additional requested headers. Let me know what you think @howard-e

Copy link
Contributor

@howard-e howard-e left a 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

stalgiag and others added 4 commits September 16, 2024 11:18
Co-authored-by: Howard Edwards <howarde.edwards@gmail.com>
…ictsTable.jsx

Co-authored-by: Howard Edwards <howarde.edwards@gmail.com>
Co-authored-by: Howard Edwards <howarde.edwards@gmail.com>
Co-authored-by: Howard Edwards <howarde.edwards@gmail.com>
@stalgiag
Copy link
Contributor Author

@howard-e I committed your comma removal editorial changes. Thank you!

@howard-e howard-e merged commit fbae626 into development Sep 17, 2024
2 checks passed
@howard-e howard-e deleted the enhanced-conflict-review branch September 17, 2024 15:42
howard-e added a commit that referenced this pull request Sep 18, 2024
…024 Release

Includes the following changes:
* #1206, to address #1202 and #1203
* #1209, to address #1204
* #1195, to address #975
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.

2 participants