accessibility-insights-web icon indicating copy to clipboard operation
accessibility-insights-web copied to clipboard

fix: Show user an "Assessment saved" dialog after they save the assessment

Open katydecorah opened this issue 3 years ago • 3 comments

Details

This pull request will add an Assessment saved dialog after the users saves an assessment. In the dialog, the user can click "Don't show again" to dismiss the dialog. Like the tab stops dialog, they can reset this value from the settings panel.

The image below shows the "Assessment save dialog" with the option to "Don't show [dialog] again" and "Got it" button to dismiss the dialog:

The image below shows the settings panel, where the user can toggle to enable the save assessment dialog: image

When building this feature, I followed the patterns with the tab stops dialog. I found one small improvement with the tab stops dialog that I made with saved assessment: use the Stack component to visually line-up the Do not show again and Got it buttons in the dialog footer. The table below shows screenshots of the current tab stops dialog and proposed:

Current Proposed
image image
Motivation

Addresses https://github.com/microsoft/accessibility-insights-web/issues/5608

Context

Pull request checklist

  • [x] Addresses an existing issue: #5608
  • [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

katydecorah avatar Jul 18 '22 19:07 katydecorah

At our lowest supported width (1280px wide by 400% zoom), the "Don't show again" text reflows kind of awkwardly:

screenshot of checkbox label reflow behavior

Ideally:

  • The checkbox label shouldn't be right-aligned
  • The checkbox should be vertically centered with the wrapped label text
  • There should be some margin in between the label text and the "Got it" button

dbjorge avatar Jul 25 '22 18:07 dbjorge

The NVDA+Edge behavior for this new dialog feels a bit awkward because the announcement of the dialog gets talked over by the browser's announcement of the download completing; the resulting readout feels to me like it's implying that our dialog controls (the "don't show again" and "got it" controls) are elements of the browser's file download UX, not a page-specific dialog.

@RobGallo, would you mind evaluating the new dialog for usability with your JAWS setup?

dbjorge avatar Jul 25 '22 18:07 dbjorge

I received @RobGallo's evaluation (a few days ago, I was focused elsewhere😅) in which he agrees that the text is confusing, but comprehendible given the constraints that we have.

Another I option I explored was the downloads.onChanged event. The component would listen for when the file downloaded and then trigger the modal. When testing with NDVA, the dialog is announced. The implementation could something like:

React.useEffect(() => {
    browser.downloads.onChanged.addListener(handleDownloadedAssessment);
    return () => {
        browser.downloads.onChanged.removeListener(handleDownloadedAssessment);
    };
}, []);

function handleDownloadedAssessment(event) {
    if (event.state && event.state.current === 'complete') {
        props.deps.detailsViewActionMessageCreator.saveAssessment(event);
        if (props.userConfigurationStoreData.showSaveAssessmentDialog) {
            showDialog();
        }
    }
}

However, this API is limited to chromium and requires manifest.json "downloads" permission. It's not clear to me if either of those are dealbreakers (or if we prefer not to use an event listener). @dbjorge what you think about using the downloads.onChanged here?

katydecorah avatar Aug 04 '22 18:08 katydecorah