Implement target date modal improvements #835
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
howard-e
requested changes
Nov 8, 2023
There was a problem hiding this comment.
@alflennik the changes look good to me other than for (5). That doesn't seem to have been addressed as yet.
From #785 (comment):
Accessibility issue: The dialog is not labeled. It has aria-labelledby, but the idref is invalid. It should reference the heading.
This is still the same. The aria-labelledby value on the div with role dialog is not referring to the heading element inside the dialog.
This PR also doesn't address (1) because you've explained that you'd like to have a follow up discussion regarding that condition. I'd be okay approving without it and adding it later if so, as it's a small change. But in the meantime, that discussion can continue in #785.
Also from #785 (comment), can support be added for:
... other bug ... the dialog does not close when escape is pressed.
@alflennik perhaps this point of feedback shouldn't block the PR after all. #837 seems to be resolving it separately. Could you confirm? cc @stalgiag |
|
Looks like you're right @howard-e! I do think I still need to fix the hide on esc. |
Yes all instances of this problem should be fixed by that PR |
|
@howard-e the escape key support has been implemented. |
stalgiag
added a commit
that referenced
this pull request
Dec 14, 2023
commit fbf8eef Author: Stalgia Grigg <stalgia@bocoup.com> Date: Thu Dec 14 14:46:48 2023 -0800 Clean up leftover import commit 15c594a Merge: 9c99b4f 56205a3 Author: Stalgia Grigg <stalgia@bocoup.com> Date: Thu Dec 14 14:29:54 2023 -0800 Merge branch 'main' into automation-mvp-merged commit 9c99b4f Author: Stalgia Grigg <stalgia@bocoup.com> Date: Thu Dec 14 14:20:15 2023 -0800 Remove aria-at git commit cut off date commit 56205a3 Author: Howard Edwards <howarde.edwards@gmail.com> Date: Thu Dec 14 11:44:34 2023 -0500 Update tests (mock data and FilterButtons.test.jsx) and formatting of frontend components (#867) commit d63609d Author: Stalgia Grigg <stalgia@bocoup.com> Date: Wed Dec 13 16:16:26 2023 -0800 Fixes after merge commit 8ff6e6b Merge: 2e62661 8215cb9 Author: Stalgia Grigg <stalgia@bocoup.com> Date: Wed Dec 13 15:03:10 2023 -0800 Merge branch 'main' into automation-mvp-rebase-attempt commit 8215cb9 Author: Stalgia Grigg <stalgia@bocoup.com> Date: Tue Dec 12 07:59:44 2023 -0800 Trap backwards navigation from initial focus on header in focus-trapped modals (#851) * Add unit test for reverse navigation from initial focused header, FocusTrapper * Update unit tests for FocusTrapper, handle backwards navigation from initially focused header * Handle focus leaving non-focusable header element backwards in FocusTrapper commit 918acdd Author: Alexander Flenniken <alflennik@users.noreply.github.com> Date: Mon Dec 11 17:20:55 2023 -0500 Allow candidate test plans to advance with override (#832) * Allow candidate test plans to advance * Implement PR feedback commit 135e61b Author: Alexander Flenniken <alflennik@users.noreply.github.com> Date: Mon Dec 11 16:59:00 2023 -0500 Polish test review v2 page (#861) * Polish test review v2 page * Remove unused variable commit 234a339 Author: Alexander Flenniken <alflennik@users.noreply.github.com> Date: Thu Dec 7 14:57:42 2023 -0500 Update CONTRIBUTING.md (#859) commit 460b29c Author: alflennik <alflennik@users.noreply.github.com> Date: Thu Dec 7 13:26:17 2023 -0500 Update contributing.md commit 457caea Author: Howard Edwards <howarde.edwards@gmail.com> Date: Tue Dec 5 16:04:23 2023 -0500 Check for v2 testFormatVersion where applicable and hide link (#855) commit 6962cef Author: Howard Edwards <howarde.edwards@gmail.com> Date: Tue Dec 5 15:36:41 2023 -0500 Support v2 Test Format in import script and page changes (#840) * Update client/resources * Update import script to support v2 test format import * Update import script to include flattened commandsV2.json and add uniquely generated scenario and assertion IDs * Updating client and server files to support V2 test format * Remove console.log and add TODO * Include tokenized assertionStatement in TestPlanVersion.tests[x].assertions * Add retrieveCommands utility * Update candidate review and datamanagement page * Update Reports/queries to also include mayOptionalAssertionResults * Add support for AtMode and and referencing appropriate screen text in testsResolver * Remove console log * Fix edge case issue when saving test result and add priority standardizing utility * Add assertions checks for may assertions * Update getMetrics utils * Update test-import to account for single {at}-focused .collected file * Address file error * Fix tests * Update import branch * Fix edge case crash when viewing CandidateTestPlanRun * Update tests and prepare support for testing v2 format test plan versions * Update tests * Update workflow * Clarifying comment * Update runtest.yml to exclude v2 test format import * Update server/resolvers/TestPlanVersion/testsResolver.js Remove unnecessary `at.settings` check Co-authored-by: Stalgia Grigg <stalgia@bocoup.com> * Clearer differences between v1 and v2 test format tests being imported * Include typedef for RenderableContent * Update thrown error message when missing commands.json (v2) * Additional check for flattenObject * Fix bug found after rebase and update jsdoc on 'retrieveAttributes' * Updating Test Run page to support #743 * Update Test Run page instructions for v2 test format * Add react-html-parser; update InstructionsRenderer * Use testPlanVersion.metadata.testFormatVersion * Stop defaulting testFormatVersion to 1 * Stop using NumberedList * Update client/components/TestRenderer/utils.js Co-authored-by: Stalgia Grigg <stalgia@bocoup.com> * Define instructions variables * Prepend tests with 'Test {number}' on reports page * Address CG comments * Address PR feedback * Consistency * Passed/failed for assertion results in TestRenderer * Update test plan report conflict calculation to support pass/fail assertion results * Correct assertion legend, margin right for checkbox * Update inline snapshot for test queue * Remove failedReason * Update server/resolvers/TestResultOperations/saveTestResultCommon.js Co-authored-by: Howard Edwards <howarde.edwards@gmail.com> * Fix linting issue in saveTestResultCommon * Undo change to checkAssertionResult * Consistency * Do radio group and supporting docs * Revert "Do radio group and supporting docs" This reverts commit 579f545. * Revert aria-at import branch --------- Co-authored-by: Paul Clue <67766160+Paul-Clue@users.noreply.github.com> Co-authored-by: Stalgia Grigg <stalgia@bocoup.com> commit 703b01f Author: jugglinmike <mike@mikepennisi.com> Date: Tue Dec 5 12:25:21 2023 -0500 Do not render zeros in place of omitted content (#852) By relying on the "truthiness" of Number values to short-circuit boolean expressions, some rendering logic produced the text "0" instead of omitting any markup at all. Replace the expressions whose value could be coerced to booleans with expressions which directly evaluate to "fallback" values that do not produce markup (i.e. `false` or an empty array). commit 7783342 Author: Stalgia Grigg <stalgia@bocoup.com> Date: Mon Dec 4 15:34:10 2023 -0800 Revert "Handle focus leaving non-focusable header element backwards in FocusTrapper" This reverts commit 57012cb. commit 57012cb Author: Stalgia Grigg <stalgia@bocoup.com> Date: Mon Dec 4 14:05:57 2023 -0800 Handle focus leaving non-focusable header element backwards in FocusTrapper commit 477abcb Author: Howard Edwards <howarde.edwards@gmail.com> Date: Wed Nov 29 10:10:10 2023 -0500 Remove additional css meant for now removed 'skipped' view (#848) commit dba7f40 Author: Alexander Flenniken <alflennik@users.noreply.github.com> Date: Tue Nov 28 14:37:24 2023 -0500 Implement Test Review in React (#846) * Implement client-side version of test-review * PR polish * Add additional specification features * Additional minor polish * PR feedback commit b98852b Author: Paul Clue <67766160+Paul-Clue@users.noreply.github.com> Date: Wed Nov 22 15:48:57 2023 -0500 No longer mark test plan reports with skipped tests as final (#833) * Hide mark as final button * No mark as final for skipped tests in graphql * Remove skipped tests from the UI * Add migration for unfinished tests * Simplify necessary files * Fix failing tests * Add code review fixes * Remove no-longer-necessary style class * Remove reference commit c29bea9 Merge: c101ecc c1bf56a Author: Stalgia Grigg <stalgia@bocoup.com> Date: Tue Nov 21 09:41:04 2023 -0800 Merge pull request #844 from w3c/fix-at-selection-dropdown Fix Data management page crash when Adding Test Plans to the Test Queue and Opening Assistive Technology select to pick any option commit c101ecc Merge: 878fe80 50cd6c8 Author: Stalgia Grigg <stalgia@bocoup.com> Date: Tue Nov 21 09:40:43 2023 -0800 Merge pull request #837 from w3c/accessible-name-test-plan-report-status-dialog Fix aria-labeledby for BasicModal/BasicThemedModal commit 878fe80 Author: Paul Clue <67766160+Paul-Clue@users.noreply.github.com> Date: Mon Nov 20 15:08:58 2023 -0500 Fix empty version strings on Data Management and Test Queue pages (#839) * Add versionString to mutation * Remove comments * Fix similar issue happening on the Test Queue page when assigning and removing testers and also removing test results --------- Co-authored-by: Howard Edwards <howarde.edwards@gmail.com> commit a5f0e80 Author: Alexander Flenniken <alflennik@users.noreply.github.com> Date: Mon Nov 20 15:01:45 2023 -0500 Implement target date modal improvements (#835) * Implement target date modal improvements * Add esc support to target date modal commit c1bf56a Author: Stalgia Grigg <stalgia@bocoup.com> Date: Thu Nov 16 11:29:56 2023 -0800 Update DataManagement test mock commit 9319e37 Author: Stalgia Grigg <stalgia@bocoup.com> Date: Thu Nov 16 11:16:37 2023 -0800 Remove browsers from query and state in TestQueue and DataManagement in favor of at.browsers commit 7dd31e1 Author: Stalgia Grigg <stalgia@bocoup.com> Date: Thu Nov 16 11:07:46 2023 -0800 Fix bug that causes crash on DataManagement page ManageTestQueue AT dropdown commit 50cd6c8 Author: Stalgia Grigg <stalgia@bocoup.com> Date: Wed Nov 8 16:01:23 2023 -0800 Unit test for aria-labeledby on BasicModal commit c1e7f0f Author: Stalgia Grigg <stalgia@bocoup.com> Date: Wed Nov 8 15:42:55 2023 -0800 Fix aria-labeledby in both BasicModal and BasicThemedModal
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements fixes listed in this issue: #785 As we discussed there are lingering questions about one of the notes so we will start a discussion about it.