chore(null): null checks for web-visualization-config-factory and deps (

#6279) #### Details This PR adds just a single file to the strict null check list (`src/common/configs/web-visualization-configuration-factory.ts`), but I've given it its own PR since one of the changes touched a lot of files. Specifically, the factory involved initializing as null several visualization fields that were present in the `VisualizationConfiguration` type but never used except by ad-hoc checks. In one case, there was a pattern that I thought was a bit misleading where a bag of UI strings called `displayableData` contained a mix of properties which all visualizations were expected to provide (`title`) vs some which only ad-hoc visualizations were expected to provide (several properties used to store the labels/etc on the ad hoc popup panel). I separated these so the ad-hoc specific ones were under a new `adHoc` grouping and included a comment on the grouping that clarified when this could be expected to be null or not. This involved touching all the files which use these properties. This isn't quite as good as using separate typings for the ad-hoc vs non-ad-hoc options to make the type system enforce that comment, but I felt it was a reasonable middle ground for how much existing code to re-write in order to make this change. ##### Motivation #2869 ##### Context n/a #### Pull request checklist  - [x] Addresses an existing issue: #2869 - [x] Ran `yarn null:autoadd` - [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`. - [n/a] (UI changes only) Added screenshots/GIFs to description above - [n/a] (UI changes only) Verified usability with NVDA/JAWS
microsoft · Dec 20, 2022 · d6eff3a · d6eff3a
1 parent 6421535
commit d6eff3a
Show file tree

Hide file tree

Showing 22 changed files with 88 additions and 57 deletions.
diff --git a/src/DetailsView/components/adhoc-static-test-view.tsx b/src/DetailsView/components/adhoc-static-test-view.tsx
@@ -44,6 +44,12 @@ export const AdhocStaticTestView = NamedFC<AdhocStaticTestViewProps>(
             !scanData.enabled,
         );
         const displayableData = props.configuration.displayableData;
+        const adHocDisplayableData = displayableData.adHoc;
+        if (adHocDisplayableData == null) {
+            throw new Error(
+                'Cannot render AdhocStaticTestView for visualization without adHoc displayableData',
+            );
+        }
 
         const stepsText = (): string => {
             const fastPassProvider = createFastPassProviderWithFeatureFlags(
@@ -68,7 +74,7 @@ export const AdhocStaticTestView = NamedFC<AdhocStaticTestViewProps>(
             visualizationEnabled: scanData.enabled,
             onToggleClick: clickHandler,
             title: displayableData.title,
-            toggleLabel: displayableData.toggleLabel,
+            toggleLabel: adHocDisplayableData.toggleLabel,
             content: props.content,
             guidance: props.guidance,
             stepsText: stepsText(),

diff --git a/src/ad-hoc-visualizations/accessible-names/visualization.tsx b/src/ad-hoc-visualizations/accessible-names/visualization.tsx
@@ -36,9 +36,11 @@ export const AccessibleNamesAdHocVisualization: VisualizationConfiguration = {
     shouldShowExportReport: () => false,
     displayableData: {
         title: 'Accessible names',
-        enableMessage: 'Calculating accessible names...',
-        toggleLabel: 'Show accessible names',
-        linkToDetailsViewText: 'How to test accessible names',
+        adHoc: {
+            enableMessage: 'Calculating accessible names...',
+            toggleLabel: 'Show accessible names',
+            linkToDetailsViewText: 'How to test accessible names',
+        },
     },
     chromeCommand: '07_toggle-accessibleNames',
     launchPanelDisplayOrder: 2,

diff --git a/src/ad-hoc-visualizations/color/visualization.tsx b/src/ad-hoc-visualizations/color/visualization.tsx
@@ -35,9 +35,11 @@ export const ColorAdHocVisualization: VisualizationConfiguration = {
     shouldShowExportReport: () => false,
     displayableData: {
         title: 'Color',
-        enableMessage: 'Changing color to grayscale...',
-        toggleLabel: 'Show grayscale',
-        linkToDetailsViewText: 'How to test color',
+        adHoc: {
+            enableMessage: 'Changing color to grayscale...',
+            toggleLabel: 'Show grayscale',
+            linkToDetailsViewText: 'How to test color',
+        },
     },
     chromeCommand: '05_toggle-color',
     launchPanelDisplayOrder: 5,

diff --git a/src/ad-hoc-visualizations/headings/visualization.tsx b/src/ad-hoc-visualizations/headings/visualization.tsx
@@ -36,9 +36,11 @@ export const HeadingsAdHocVisualization: VisualizationConfiguration = {
     shouldShowExportReport: () => false,
     displayableData: {
         title: 'Headings',
-        enableMessage: 'Finding headings...',
-        toggleLabel: 'Show headings',
-        linkToDetailsViewText: 'How to test headings',
+        adHoc: {
+            enableMessage: 'Finding headings...',
+            toggleLabel: 'Show headings',
+            linkToDetailsViewText: 'How to test headings',
+        },
     },
     chromeCommand: '03_toggle-headings',
     launchPanelDisplayOrder: 3,

diff --git a/src/ad-hoc-visualizations/issues/visualization.tsx b/src/ad-hoc-visualizations/issues/visualization.tsx
@@ -49,9 +49,11 @@ export const IssuesAdHocVisualization: VisualizationConfiguration = {
                 .
             </>
         ),
-        enableMessage: 'Running automated checks...',
-        toggleLabel: 'Show failures',
-        linkToDetailsViewText: 'List view and filtering',
+        adHoc: {
+            enableMessage: 'Running automated checks...',
+            toggleLabel: 'Show failures',
+            linkToDetailsViewText: 'List view and filtering',
+        },
     },
     chromeCommand: '01_toggle-issues',
     launchPanelDisplayOrder: 1,

diff --git a/src/ad-hoc-visualizations/landmarks/visualization.tsx b/src/ad-hoc-visualizations/landmarks/visualization.tsx
@@ -36,9 +36,11 @@ export const LandmarksAdHocVisualization: VisualizationConfiguration = {
     shouldShowExportReport: () => false,
     displayableData: {
         title: 'Landmarks',
-        enableMessage: 'Finding landmarks...',
-        toggleLabel: 'Show landmarks',
-        linkToDetailsViewText: 'How to test landmarks',
+        adHoc: {
+            enableMessage: 'Finding landmarks...',
+            toggleLabel: 'Show landmarks',
+            linkToDetailsViewText: 'How to test landmarks',
+        },
     },
     chromeCommand: '02_toggle-landmarks',
     launchPanelDisplayOrder: 2,

diff --git a/src/ad-hoc-visualizations/needs-review/visualization.tsx b/src/ad-hoc-visualizations/needs-review/visualization.tsx
@@ -48,9 +48,11 @@ export const NeedsReviewAdHocVisualization: VisualizationConfiguration = {
                 .
             </>
         ),
-        enableMessage: 'Running needs review checks...',
-        toggleLabel: 'Show elements needing review',
-        linkToDetailsViewText: 'List view and filtering',
+        adHoc: {
+            enableMessage: 'Running needs review checks...',
+            toggleLabel: 'Show elements needing review',
+            linkToDetailsViewText: 'List view and filtering',
+        },
     },
     chromeCommand: '06_toggle-needsReview',
     launchPanelDisplayOrder: 6,

diff --git a/src/ad-hoc-visualizations/tab-stops/visualization.tsx b/src/ad-hoc-visualizations/tab-stops/visualization.tsx
@@ -37,9 +37,11 @@ export const TabStopsAdHocVisualization: VisualizationConfiguration = {
     shouldShowExportReport: () => true,
     displayableData: {
         title: 'Tab stops',
-        enableMessage: 'Start pressing Tab to start visualizing tab stops.',
-        toggleLabel: 'Show tab stops',
-        linkToDetailsViewText: 'How to test tab stops',
+        adHoc: {
+            enableMessage: 'Start pressing Tab to start visualizing tab stops.',
+            toggleLabel: 'Show tab stops',
+            linkToDetailsViewText: 'How to test tab stops',
+        },
     },
     chromeCommand: '04_toggle-tabStops',
     launchPanelDisplayOrder: 4,

diff --git a/src/background/keyboard-shortcut-handler.ts b/src/background/keyboard-shortcut-handler.ts
@@ -135,9 +135,9 @@ export class KeyboardShortcutHandler {
         if (!scanData.enabled) {
             const displayableVisualizationData = configuration.displayableData;
 
-            if (displayableVisualizationData) {
+            if (displayableVisualizationData?.adHoc?.enableMessage) {
                 this.notificationCreator.createNotification(
-                    displayableVisualizationData.enableMessage,
+                    displayableVisualizationData.adHoc.enableMessage,
                 );
             }
         }

diff --git a/src/common/configs/visualization-configuration-factory.ts b/src/common/configs/visualization-configuration-factory.ts
@@ -10,7 +10,7 @@ import { VisualizationConfiguration } from './visualization-configuration';
 // The interface is split to avoid circular dependencies (this interface is a commonly used react
 // prop/dep for components which WebVisualizationConfigurationFactory indirectly points to)
 export interface VisualizationConfigurationFactory {
-    getConfigurationByKey(key: string): VisualizationConfiguration;
+    getConfigurationByKey(key: string): VisualizationConfiguration | undefined;
     getConfiguration(visualizationType: VisualizationType): VisualizationConfiguration;
     getChromeCommandToVisualizationTypeMap(): DictionaryStringTo<VisualizationType>;
     forEachConfig(callback: ForEachConfigCallback): void;

diff --git a/src/common/configs/visualization-configuration.ts b/src/common/configs/visualization-configuration.ts
@@ -21,9 +21,9 @@ export interface VisualizationConfiguration extends AssessmentVisualizationConfi
         instanceMap?: DictionaryStringTo<any>,
     ) => void;
     displayableData: DisplayableVisualizationTypeData;
-    chromeCommand: string;
-    launchPanelDisplayOrder: number;
-    adhocToolsPanelDisplayOrder: number;
+    chromeCommand: string | null;
+    launchPanelDisplayOrder: number | null;
+    adhocToolsPanelDisplayOrder: number | null;
     analyzerProgressMessageType?: string;
     analyzerTerminatedMessageType?: string;
     guidance?: ContentPageComponent;

diff --git a/src/common/configs/web-visualization-configuration-factory.ts b/src/common/configs/web-visualization-configuration-factory.ts
@@ -43,13 +43,13 @@ export class WebVisualizationConfigurationFactory implements VisualizationConfig
         };
     }
 
-    public getConfigurationByKey(key: string): VisualizationConfiguration {
+    public getConfigurationByKey(key: string): VisualizationConfiguration | undefined {
         return find(values(this.configurationByType), config => config.key === key);
     }
 
     public getConfiguration(visualizationType: VisualizationType): VisualizationConfiguration {
         if (this.mediumPassProvider?.isValidType(visualizationType)) {
-            const assessment = this.mediumPassProvider.forType(visualizationType);
+            const assessment = this.mediumPassProvider.forType(visualizationType)!;
             return this.buildAssessmentConfiguration(
                 assessment,
                 TestMode.MediumPass,
@@ -58,7 +58,7 @@ export class WebVisualizationConfigurationFactory implements VisualizationConfig
         }
 
         if (this.fullAssessmentProvider.isValidType(visualizationType)) {
-            const assessment = this.fullAssessmentProvider.forType(visualizationType);
+            const assessment = this.fullAssessmentProvider.forType(visualizationType)!;
             return this.buildAssessmentConfiguration(
                 assessment,
                 TestMode.Assessments,
@@ -114,7 +114,7 @@ export class WebVisualizationConfigurationFactory implements VisualizationConfig
 
         const getIdentifier = (requirementKey: string) => {
             const requirement = assessment.requirements.find(req => req.key === requirementKey);
-            return `${testMode}-${requirement.key}`;
+            return `${testMode}-${requirement!.key}`;
         };
 
         const getStoreData = (data: TestsEnabledState) => {
@@ -128,12 +128,9 @@ export class WebVisualizationConfigurationFactory implements VisualizationConfig
             adhocToolsPanelDisplayOrder: null,
             displayableData: {
                 title: assessment.title,
-                noResultsFound: null,
-                enableMessage: null,
-                toggleLabel: null,
-                linkToDetailsViewText: null,
+                adHoc: null,
             },
-            shouldShowExportReport: null,
+            shouldShowExportReport: () => false,
             getIdentifier,
             getStoreData,
             messageConfiguration,

diff --git a/src/common/types/displayable-visualization-type-data.ts b/src/common/types/displayable-visualization-type-data.ts
@@ -3,6 +3,12 @@
 export interface DisplayableVisualizationTypeData {
     title: string;
     subtitle?: JSX.Element;
+
+    // populated if-and-only-if visualization is for an ad-hoc tool
+    adHoc: AdHocDisplayableVisualizationTypeData | null;
+}
+
+export interface AdHocDisplayableVisualizationTypeData {
     enableMessage: string;
     toggleLabel: string;
     linkToDetailsViewText: string;

diff --git a/src/popup/components/diagnostic-view-toggle.tsx b/src/popup/components/diagnostic-view-toggle.tsx
@@ -68,13 +68,21 @@ export class DiagnosticViewToggle extends React.Component<
             <div className={styles.diagnosticViewToggle}>
                 <div className={styles.title}>{displayableData.title}</div>
                 <div className={styles.toggle}>{this.renderToggleOrSpinner()}</div>
-                <div>{this.renderLink(displayableData.linkToDetailsViewText)}</div>
+                {this.renderLinkToDetailsView()}
                 {this.renderShortcut()}
             </div>
         );
     }
 
-    private renderShortcut(): JSX.Element {
+    private renderLinkToDetailsView(): JSX.Element | null {
+        const linkText = this.configuration.displayableData.adHoc?.linkToDetailsViewText;
+        if (linkText == null) {
+            return null;
+        }
+        return <div>{this.renderLink(linkText)}</div>;
+    }
+
+    private renderShortcut(): JSX.Element | null {
         if (!this.configuration.chromeCommand) {
             return null;
         }

diff --git a/...s/unit/tests/DetailsView/components/__snapshots__/adhoc-tab-stops-test-view.test.tsx.snap b/...s/unit/tests/DetailsView/components/__snapshots__/adhoc-tab-stops-test-view.test.tsx.snap
@@ -278,8 +278,10 @@ exports[`AdhocTabStopsTestView render should return target page changed view as
   detailsViewActionMessageCreator={[Function]}
   displayableData={
     {
+      "adHoc": {
+        "toggleLabel": "test toggle label",
+      },
       "title": "test title",
-      "toggleLabel": "test toggle label",
     }
   }
   toggleClickHandler={[Function]}

diff --git a/src/tests/unit/tests/DetailsView/components/adhoc-static-test-view.test.tsx b/src/tests/unit/tests/DetailsView/components/adhoc-static-test-view.test.tsx
@@ -38,7 +38,9 @@ describe('AdhocStaticTestView', () => {
         );
         displayableDataStub = {
             title: 'test title',
-            toggleLabel: 'test toggle label',
+            adHoc: {
+                toggleLabel: 'test toggle label',
+            },
         } as DisplayableVisualizationTypeData;
         scanDataStub = {
             enabled: true,

diff --git a/src/tests/unit/tests/DetailsView/components/adhoc-tab-stops-test-view.test.tsx b/src/tests/unit/tests/DetailsView/components/adhoc-tab-stops-test-view.test.tsx
@@ -46,7 +46,9 @@ describe('AdhocTabStopsTestView', () => {
         );
         displayableDataStub = {
             title: 'test title',
-            toggleLabel: 'test toggle label',
+            adHoc: {
+                toggleLabel: 'test toggle label',
+            },
         } as DisplayableVisualizationTypeData;
         scanDataStub = {
             enabled: true,

diff --git a/src/tests/unit/tests/DetailsView/components/details-list-issues-view.test.tsx b/src/tests/unit/tests/DetailsView/components/details-list-issues-view.test.tsx
@@ -33,7 +33,9 @@ describe('DetailsListIssuesView', () => {
         displayableDataStub = {
             title: 'test title',
             subtitle: <>test subtitle</>,
-            toggleLabel: 'test toggle label',
+            adHoc: {
+                toggleLabel: 'test toggle label',
+            },
         } as DisplayableVisualizationTypeData;
         scanDataStub = {
             enabled: true,

diff --git a/src/tests/unit/tests/DetailsView/components/target-page-changed-view.test.tsx b/src/tests/unit/tests/DetailsView/components/target-page-changed-view.test.tsx
@@ -23,7 +23,9 @@ describe('TargetPageChangedView', () => {
             const clickHandlerStub: () => void = () => {};
             const displayableData = {
                 title: 'test title',
-                toggleLabel: 'test toggle label',
+                adHoc: {
+                    toggleLabel: 'test toggle label',
+                },
                 subtitle,
             } as DisplayableVisualizationTypeData;
             const detailsViewActionMessageCreator = {} as DetailsViewActionMessageCreator;

diff --git a/src/tests/unit/tests/background/keyboard-shortcut-handler.test.ts b/src/tests/unit/tests/background/keyboard-shortcut-handler.test.ts
@@ -244,7 +244,7 @@ describe('KeyboardShortcutHandler', () => {
             const configuration =
                 visualizationConfigurationFactory.getConfiguration(visualizationType);
 
-            const enableMessage = configuration.displayableData.enableMessage;
+            const enableMessage = configuration.displayableData.adHoc.enableMessage;
             notificationCreatorMock
                 .setup(nc => nc.createNotification(enableMessage))
                 .verifiable(Times.once());

diff --git a/src/tests/unit/tests/common/configs/web-visualization-configuration-factory.test.ts b/src/tests/unit/tests/common/configs/web-visualization-configuration-factory.test.ts
@@ -356,12 +356,8 @@ describe('WebVisualizationConfigurationFactory', () => {
             adhocToolsPanelDisplayOrder: null,
             displayableData: {
                 title: expectedDisplayableTitle,
-                noResultsFound: null,
-                enableMessage: null,
-                toggleLabel: null,
-                linkToDetailsViewText: null,
+                adHoc: null,
             },
-            shouldShowExportReport: null,
             messageConfiguration: expectedMessageConfig,
         };
     }

diff --git a/tsconfig.strictNullChecks.json b/tsconfig.strictNullChecks.json
@@ -239,13 +239,6 @@
         "./src/common/components/toast.tsx",
         "./src/common/components/visualization-toggle.tsx",
         "./src/common/components/with-store-subscription.tsx",
-        "./src/common/configs/a11y-insights-rule-resources.ts",
-        "./src/common/configs/assessment-visualization-configuration.ts",
-        "./src/common/configs/inspect-configuration-factory.ts",
-        "./src/common/configs/test-mode.ts",
-        "./src/common/configs/unified-result-property-configurations.tsx",
-        "./src/common/configs/visualization-configuration-factory.ts",
-        "./src/common/configs/visualization-configuration.ts",
         "./src/common/date-provider.ts",
         "./src/common/document-manipulator.ts",
         "./src/common/dropdown-click-handler.ts",
@@ -578,6 +571,7 @@
         "src/common/action/**/*",
         "src/common/browser-adapters/**/*",
         "src/common/components/**/*",
+        "src/common/configs/**/*",
         "src/common/configuration/**/*",
         "src/common/constants/**/*",
         "src/common/flux/**/*",