Skip to content

Commit

Permalink
chore(null): strict null checks for more injected/visualization files (
Browse files Browse the repository at this point in the history
…#6234)

#### Details

This PR continues from #6223 and fixes another few layers of
`injected/visualization` files to be strict null check compatible. It
substantially refactors `accessibile-name-formatter` and somewhat
refactors `landmark-formatter` - I manually smoke tested those ad hoc
tools to verify that they still worked after the changes

##### Motivation

#2869

##### Context

n/a

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a"
in the checkbox -->
- [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
  • Loading branch information
dbjorge authored Dec 5, 2022
1 parent 432f474 commit aece58e
Show file tree
Hide file tree
Showing 15 changed files with 193 additions and 163 deletions.
6 changes: 3 additions & 3 deletions src/common/target-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ export class TargetHelper {
target: (string | string[])[],
dom: Document,
targetIndex,
) => {
): Element[] => {
if (!target || target.length < 1) {
return;
return [];
}
const selectors = target[targetIndex];
let elements: NodeListOf<Element> | null;
Expand All @@ -32,7 +32,7 @@ export class TargetHelper {
} else {
const shadowHost = this.getShadowHost(selectors, dom);
if (!shadowHost || !shadowHost.shadowRoot) {
return;
return [];
}
elements = shadowHost.shadowRoot.querySelectorAll(selectors[selectors.length - 1]);
}
Expand Down
59 changes: 19 additions & 40 deletions src/injected/visualization/accessible-names-formatter.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import {
FormattedCheckResult,
HtmlElementAxeResults,
} from 'common/types/store-data/visualization-scan-result-data';
import { HtmlElementAxeResults } from 'common/types/store-data/visualization-scan-result-data';
import { DialogRenderer } from 'injected/dialog-renderer';
import { AssessmentVisualizationInstance } from 'injected/frameCommunicators/html-element-axe-results-helper';
import { DrawerConfiguration, Formatter } from 'injected/visualization/formatter';
import { HeadingStyleConfiguration } from 'injected/visualization/heading-formatter';
interface DisplayAccessibleNameData {
accessibleName: string;
}

export class AccessibleNamesFormatter implements Formatter {
public getDialogRenderer(): DialogRenderer {
public getDialogRenderer(): DialogRenderer | null {
return null;
}

Expand All @@ -21,35 +16,28 @@ export class AccessibleNamesFormatter implements Formatter {
fontColor: '#FFFFFF',
};

public getData(nodes: FormattedCheckResult[]) {
for (const check of nodes) {
if (check.id === 'display-accessible-names') {
return {
accessibleName: check.data.accessibleName,
};
}
}
}
private accessibleNameFromCheck(data: HtmlElementAxeResults | null): string | null {
const ruleResult = data?.ruleResults['display-accessible-names'];
const checkResult = ruleResult?.any?.find(check => check.id === 'display-accessible-names');

private getInfo(data: HtmlElementAxeResults): DisplayAccessibleNameData {
for (const idx in data.ruleResults) {
if (data.ruleResults[idx].ruleId === 'display-accessible-names') {
return this.getData(data.ruleResults[idx].any);
}
}
return undefined;
return checkResult?.data?.accessibleName ?? null;
}

public getDrawerConfiguration(
element: HTMLElement,
data: AssessmentVisualizationInstance,
data: AssessmentVisualizationInstance | null,
): DrawerConfiguration {
const accessibleNameToDisplay = this.formatText(this.getInfo(data));
const accessibleName = this.accessibleNameFromCheck(data);
if (accessibleName == null) {
return { showVisualization: false };
}

const formattedAccessibleName = this.formatAccessibleName(accessibleName);
const config: DrawerConfiguration = {
textBoxConfig: {
fontColor: AccessibleNamesFormatter.style.fontColor,
background: AccessibleNamesFormatter.style.outlineColor,
text: accessibleNameToDisplay?.accessibleName,
text: formattedAccessibleName,
fontWeight: '400',
fontSize: '10px',
outline: '3px dashed',
Expand All @@ -62,21 +50,12 @@ export class AccessibleNamesFormatter implements Formatter {
return config;
}

private formatText(accessibleNameData: DisplayAccessibleNameData): DisplayAccessibleNameData {
const ElmtAccessibleName = accessibleNameData?.accessibleName;
let nameToDisplay;
private formatAccessibleName(accessibleName: string): string {
const allowedNameMaxLength = 40;
if (ElmtAccessibleName === undefined) {
nameToDisplay = undefined;
if (accessibleName.length <= allowedNameMaxLength) {
return accessibleName;
} else {
if (ElmtAccessibleName.length <= allowedNameMaxLength) {
nameToDisplay = ElmtAccessibleName;
} else if (ElmtAccessibleName.length > allowedNameMaxLength) {
nameToDisplay = `${ElmtAccessibleName.substring(0, allowedNameMaxLength)}...`;
}
return `${accessibleName.substring(0, allowedNameMaxLength)}...`;
}
return {
accessibleName: nameToDisplay,
};
}
}
2 changes: 1 addition & 1 deletion src/injected/visualization/base-drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export abstract class BaseDrawer implements Drawer {
protected containerClass: string;
protected changeHandler: () => void;
public static recalculationTimeout = 16;
protected dialogRenderer: DialogRenderer;
protected dialogRenderer: DialogRenderer | null;
protected windowUtils: WindowUtils;
protected containerElement: HTMLElement | null;
protected drawerUtils: DrawerUtils;
Expand Down
30 changes: 15 additions & 15 deletions src/injected/visualization/focus-indicator-creator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ export class FocusIndicatorCreator {
prevItem: TabbedItem,
isLastItem: boolean,
formatter: Formatter,
): FocusIndicator => {
const centerPosition: Point = this.centerPositionCalculator.getElementCenterPosition(
): FocusIndicator | null => {
const centerPosition: Point | null = this.centerPositionCalculator.getElementCenterPosition(
item.element,
);

if (centerPosition == null) {
return;
if (item.element == null || centerPosition == null) {
return null;
}

const drawerConfig = formatter.getDrawerConfiguration(
Expand Down Expand Up @@ -69,20 +69,20 @@ export class FocusIndicatorCreator {
drawerConfig.tabIndexLabel,
item.tabOrder.toString(),
);
const newLine: Element = this.createLinesInTabOrderVisualization(
const newLine: Element | null = this.createLinesInTabOrderVisualization(
item,
prevItem,
drawerConfig.line,
centerPosition,
drawerConfig.circle.ellipseRx,
drawerConfig.line.showSolidFocusLine,
drawerConfig.line.showSolidFocusLine ?? false,
);

const showTabIndexedLabel = drawerConfig.tabIndexLabel.showTabIndexedLabel;
const focusIndicator: FocusIndicator = {
circle: newCircle,
tabIndexLabel: !showTabIndexedLabel ? null : newLabel,
line: newLine,
tabIndexLabel: !showTabIndexedLabel ? undefined : newLabel,
line: newLine ?? undefined,
};

return focusIndicator;
Expand All @@ -95,7 +95,7 @@ export class FocusIndicatorCreator {
centerPosition: Point,
ellipseRx: string,
showSolidFocusLine: boolean,
): Element {
): Element | null {
const shouldBreakGraph = this.shouldBreakGraph(item, prevItem);

if (!showSolidFocusLine || shouldBreakGraph) {
Expand Down Expand Up @@ -135,23 +135,23 @@ export class FocusIndicatorCreator {
drawerConfig.focusedLine,
elementPosition,
drawerConfig.focusedCircle.ellipseRx,
drawerConfig.line.showSolidFocusLine,
drawerConfig.line.showSolidFocusLine ?? false,
);
return {
circle: lastItemCircle,
line: lastItemLine,
tabIndexLabel: null,
line: lastItemLine ?? undefined,
tabIndexLabel: undefined,
};
};

public createFocusIndicatorForFailure = (
item: TabbedItem,
formatter: Formatter,
): FocusIndicator => {
): FocusIndicator | null => {
const centerPosition = this.centerPositionCalculator.getElementCenterPosition(item.element);

if (centerPosition == null) {
return;
if (item.element == null || centerPosition == null) {
return null;
}

const drawerConfig = formatter.getDrawerConfiguration(
Expand Down
2 changes: 1 addition & 1 deletion src/injected/visualization/formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export interface SingleTargetDrawerConfiguration {
export interface Formatter {
getDrawerConfiguration(
element: Node,
data: AxeResultsWithFrameLevel,
data: AxeResultsWithFrameLevel | null,
): DrawerConfiguration | SVGDrawerConfiguration | SingleTargetDrawerConfiguration;
getDialogRenderer(): DialogRenderer | null;
}
2 changes: 1 addition & 1 deletion src/injected/visualization/get-cell-and-header-elements.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const getCellAndHeaderElementsFromResult = (
if (isEmpty(headersAttr)) {
return;
}
const headers = headersAttr.split(' ');
const headers = headersAttr!.split(' ');
headers.forEach(headerId => {
const headerElement = dom.getElementById(headerId);
if (headerElement) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { TabStopsScanResultData } from 'common/types/store-data/visualization-scan-result-data';
import {
SingleTabStopRequirementState,
TabStopsScanResultData,
} from 'common/types/store-data/visualization-scan-result-data';
import { TabStopVisualizationInstance } from 'injected/frameCommunicators/html-element-axe-results-helper';
import {
InstanceIdToTabStopVisualizationMap,
Expand All @@ -24,7 +27,7 @@ export const GetVisualizationInstancesForTabStops = (
const instance: TabStopVisualizationInstance = {
isFailure: false,
isVisualizationEnabled: true,
ruleResults: null,
ruleResults: {},
target: element.target,
propertyBag: {
tabOrder: element.tabOrder,
Expand All @@ -36,34 +39,37 @@ export const GetVisualizationInstancesForTabStops = (
instanceIdToVisualizationInstanceMap[element.instanceId] = instance;
});

forOwn(tabStopScanResultData.requirements, (obj, requirementId: TabStopRequirementId) => {
obj.instances.forEach(instance => {
if (instance.selector == null) {
return;
}
forOwn(
tabStopScanResultData.requirements,
(obj: SingleTabStopRequirementState, requirementId: TabStopRequirementId) => {
obj.instances.forEach(instance => {
if (instance.selector == null) {
return;
}

const itemType =
requirementId === 'keyboard-navigation'
? TabbedItemType.MissingItem
: TabbedItemType.ErroredItem;
const itemType =
requirementId === 'keyboard-navigation'
? TabbedItemType.MissingItem
: TabbedItemType.ErroredItem;

const newInstance: TabStopVisualizationInstance = {
isFailure: true,
isVisualizationEnabled: true,
ruleResults: null,
target: instance.selector,
propertyBag: {},
requirementResults: {
[requirementId]: {
instanceId: instance.id,
const newInstance: TabStopVisualizationInstance = {
isFailure: true,
isVisualizationEnabled: true,
ruleResults: {},
target: instance.selector,
propertyBag: {},
requirementResults: {
[requirementId]: {
instanceId: instance.id,
},
},
},
itemType,
};
itemType,
};

instanceIdToVisualizationInstanceMap[instance.id] = newInstance;
});
});
instanceIdToVisualizationInstanceMap[instance.id] = newInstance;
});
},
);

return instanceIdToVisualizationInstanceMap;
};
29 changes: 17 additions & 12 deletions src/injected/visualization/highlight-box-drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,15 @@ const getTargetElementsFromResult = (result: AxeResultsWithFrameLevel, dom: Docu

export class HighlightBoxDrawer extends BaseDrawer {
protected elementResults: AxeResultsWithFrameLevel[];
protected dialogRenderer: DialogRenderer;
private clientUtils: ClientUtils;
protected readonly dialogRenderer: DialogRenderer | null;

constructor(
dom: Document,
containerClass: string,
windowUtils: WindowUtils,
shadowUtils: ShadowUtils,
drawerUtils: DrawerUtils,
clientUtils: ClientUtils,
private readonly clientUtils: ClientUtils,
formatter: Formatter,
private readonly getElementsToHighlight: typeof getTargetElementsFromResult = getTargetElementsFromResult,
) {
Expand All @@ -45,7 +44,7 @@ export class HighlightBoxDrawer extends BaseDrawer {
protected addHighlightsToContainer = async (): Promise<void> => {
const highlightElements = await this.getHighlightElements();

if (highlightElements.length > 0) {
if (this.containerElement != null && highlightElements.length > 0) {
for (let elementPos = 0; elementPos < highlightElements.length; elementPos++) {
this.containerElement.appendChild(highlightElements[elementPos]);
}
Expand All @@ -55,7 +54,7 @@ export class HighlightBoxDrawer extends BaseDrawer {
protected createHighlightElement = async (
element: Element,
data: HtmlElementAxeResults,
): Promise<HTMLElement> => {
): Promise<HTMLElement | undefined> => {
const currentDom = this.drawerUtils.getDocumentElement();
const body = currentDom.body;
const bodyStyle = this.windowUtils.getComputedStyle(body);
Expand Down Expand Up @@ -91,7 +90,9 @@ export class HighlightBoxDrawer extends BaseDrawer {
const wrapper = currentDom.createElement('div');
wrapper.classList.add('insights-highlight-box');
wrapper.classList.add(`insights-highlight-outline-${drawerConfig.outlineStyle ?? 'solid'}`);
wrapper.style.outlineColor = drawerConfig.outlineColor;
if (drawerConfig.outlineColor != null) {
wrapper.style.outlineColor = drawerConfig.outlineColor;
}

wrapper.style.top = this.drawerUtils.getContainerTopOffset(offset).toString() + 'px';
wrapper.style.left = this.drawerUtils.getContainerLeftOffset(offset).toString() + 'px';
Expand Down Expand Up @@ -138,7 +139,7 @@ export class HighlightBoxDrawer extends BaseDrawer {

if (drawerConfig.failureBoxConfig.hasDialogView) {
failureBox.addEventListener('click', async () => {
await this.dialogRenderer.render(data as any);
await this.dialogRenderer?.render(data as any);
});
}
wrapper.appendChild(failureBox);
Expand All @@ -159,11 +160,15 @@ export class HighlightBoxDrawer extends BaseDrawer {
box.innerText = boxConfig.text || '';
box.style.background = boxConfig.background;
box.style.color = boxConfig.fontColor;
box.style.fontSize = boxConfig.fontSize;
box.style.fontWeight = boxConfig.fontWeight;
box.style.setProperty('width', boxConfig.boxWidth, 'important');
box.style.setProperty('cursor', drawerConfig.cursor, 'important');
box.style.setProperty('text-align', drawerConfig.textAlign, 'important');
if (boxConfig.fontSize != null) {
box.style.fontSize = boxConfig.fontSize;
}
if (boxConfig.fontWeight != null) {
box.style.fontWeight = boxConfig.fontWeight;
}
box.style.setProperty('width', boxConfig.boxWidth ?? null, 'important');
box.style.setProperty('cursor', drawerConfig.cursor ?? null, 'important');
box.style.setProperty('text-align', drawerConfig.textAlign ?? null, 'important');

return box;
}
Expand Down
Loading

0 comments on commit aece58e

Please sign in to comment.