Skip to content

Commit

Permalink
fix: Make failure highlight more visible (#6164)
Browse files Browse the repository at this point in the history
#### Details

<!-- Usually a sentence or two describing what the PR changes -->

##### Motivation

<!-- This can be as simple as "addresses issue #123" -->
close #5610

##### Context

<!-- Are there any parts that you've intentionally left out-of-scope for
a later PR to handle? -->

<!-- Were there any alternative approaches you considered? What
tradeoffs did you consider? -->

> Design team suggested trying multiple border colors with red in the
center and white surrounding this.

I implemented this based on one of the design team's suggestions listed
in the issue.

This will make failure highlight more visible, especially for a dark
background as in the following picture. However, for pages with white or
other background colors, it is not very effective.

##### screenshots/GIFs to description above

Dark Background
| Before | After |
| ----- | ----- |
|<img width="1267" alt="スクリーンショット 2022-11-03 20 00 00"
src="https://user-images.githubusercontent.com/49313042/199704496-45e60ebe-4818-46a2-9203-c5d0364fd1f2.png">|<img
width="1245" alt="スクリーンショット 2022-11-03 19 58 34"
src="https://user-images.githubusercontent.com/49313042/199704535-48475702-dd3a-4b62-83ce-750d17137731.png">|

White Background
| Before | After |
| ----- | ----- |
|<img width="1015" alt="スクリーンショット 2022-11-03 20 14 26"
src="https://user-images.githubusercontent.com/49313042/199707053-d05a56dc-be96-4a81-9fb8-c8e7ab1cc3d4.png">|<img
width="1141" alt="スクリーンショット 2022-11-03 20 11 11"
src="https://user-images.githubusercontent.com/49313042/199706541-03591b81-7287-4dd1-86c5-d437a87bc5fd.png">|

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a"
in the checkbox -->
- [x] Addresses an existing issue: #5610
- [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`.
- [x] (UI changes only) Added screenshots/GIFs to description above
- [x] (UI changes only) Verified usability with NVDA/JAWS

---

I've pushed an update that swaps to the box-shadow + outline version. It updates all the box drawers and their labels. It pushes the determination of box width to CSS to reduce the amount of per-element overhead in `style` attributes. It touches a bunch of files to change some `DrawerConfiguration` types from using `borderColor` to `outlineColor`. I also made some related refactorings while patching up the tests:

* The `HighlightBoxDrawer` previously allowed for no formatter to be passed and contained a "default drawer configuration" for that case. This was never used except for tests; I've removed it and updated `drawer.test.ts` accordingly
* Removed an obsolete `LandmarkValue` type (not used anywhere and referenced `borderColor`)

I also fixed up some issues specific to the landmark visualizer; per discussion with @ferBonnin, I included the same white border there but reduced the width of the dashed part from 3px to 2px. I also fixed an issue I noticed with high-contrast-mode rendering of the landmark visualizer's dashed highlight boxes (previously, we used HC colors for the outer box, but not the text label box; this fixes it to be consistent).

For reviewers, I recommend including the Deque Mars test page among your smoke tests; it has a lot of failures against non-white backgrounds that are good test cases.

Screenshots of new behavior:

![ad-hoc automated checks](https://user-images.githubusercontent.com/376284/202323774-c748503a-1dff-442a-bc3a-0c29a81f0119.png)

![ad-hoc headings](https://user-images.githubusercontent.com/376284/202323853-6fdd3033-0a9d-4ea9-9431-5ba09fece587.png)

![ad-hoc landmarks](https://user-images.githubusercontent.com/376284/202323899-4f326328-15fa-498b-a71a-8c6d4e0f896b.png)

Tab stops is not changed by this update and would need to be addressed in a separate PR: 
![tab stops](https://user-images.githubusercontent.com/376284/202324015-d68a336a-99a3-4b4a-9cc1-9edfe02a8d3d.png)

In Windows HC mode:

![automated checks visualizer in Windows high contrast Aquatic](https://user-images.githubusercontent.com/376284/202324100-16289224-d924-452e-9d98-ba4b093632dc.png)

![landmarks visualizer in Windows high contrast Aquatic](https://user-images.githubusercontent.com/376284/202328516-228ff77c-b564-4f19-b563-9163a75707ed.png)

Co-authored-by: Dan Bjorge <[email protected]>
  • Loading branch information
segamiken and dbjorge authored Nov 22, 2022
1 parent c7d4e38 commit b34426b
Show file tree
Hide file tree
Showing 27 changed files with 209 additions and 180 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export function headingsAssessmentInstanceDetailsColumnRenderer(
const headingLevel = propertyBag ? propertyBag.headingLevel : null;
const labelText = headingLevel ? `H${item.instance.propertyBag.headingLevel}` : 'N/A';
const headingStyle = headingLevel ? HeadingFormatter.headingStyles[headingLevel] : null;
const background = headingStyle ? headingStyle.borderColor : '#767676';
const background = headingStyle ? headingStyle.outlineColor : '#767676';
let customClass: string = null;

if (headingLevel == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export function landmarksAssessmentInstanceDetailsColumnRenderer(
item: InstanceTableRow<LandmarksAssessmentProperties>,
): JSX.Element {
const propertyBag = item.instance.propertyBag;
const background = LandmarkFormatter.getStyleForLandmarkRole(propertyBag.role).borderColor;
const background = LandmarkFormatter.getStyleForLandmarkRole(propertyBag.role).outlineColor;
let textContent = propertyBag.role;
if (propertyBag.label != null) {
textContent += `: ${propertyBag.label}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function frameTitleInstanceDetailsColumnRenderer(

return (
<AssessmentInstanceDetailsColumn
background={frameConfig.borderColor}
background={frameConfig.outlineColor}
labelText={frameConfig.contentText}
textContent={frameTitle}
/>
Expand Down
7 changes: 0 additions & 7 deletions src/injected/scanner.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,6 @@ declare interface AxeNodeResult {
snippet?: string;
}

declare interface LandmarkValue {
selectors: string[];
label: string[];
borderColor: string;
fontColor: string;
}

declare function getAxeResults(
rulesToTest: string[],
successCallback: (axeResults: AxeResult) => void,
Expand Down
82 changes: 59 additions & 23 deletions src/injected/styles/injected.scss
Original file line number Diff line number Diff line change
Expand Up @@ -255,34 +255,70 @@
}
}

.insights-highlight-container .insights-highlight-box {
visibility: visible !important;
position: absolute !important;
overflow: hidden !important;
pointer-events: none !important;
.insights-highlight-container {
.insights-highlight-box {
visibility: visible !important;
position: absolute !important;
overflow: hidden !important;
pointer-events: none !important;
}

@include ms-high-contrast-override {
forced-color-adjust: none;
outline-color: Highlight !important;
// This applies to both the text in highlight boxes and also the text
// in highlight circled used for tab/focus visualizers
.insights-highlight-text {
@include ms-high-contrast-override {
// Note that forced-color-adjust: none is required for highlight box labels'
// outline/box-shadows to be inherited correctly in HC mode, in addition to being
// required for color and background-color
forced-color-adjust: none;
color: HighlightText !important;
background-color: Highlight !important;
}
}
}

.insights-highlight-container .insights-highlight-text {
@include ms-high-contrast-override {
forced-color-adjust: none;
color: HighlightText !important;
background-color: Highlight !important;
.insights-highlight-box .insights-highlight-text {
cursor: default !important;
pointer-events: all !important;
font-size: 1em !important;
padding: 0.3em !important;
float: right !important;
position: relative !important;
text-align: center !important;

// This results in the highlight box outline appearing to also run down the left edge of
// the label
outline: inherit !important;
outline-offset: inherit !important;
box-shadow: inherit !important;
}

.insights-highlight-outline-solid {
// Non-HC outline-color is intentionally unset; individual formatters should override it
outline-width: 2px !important;
outline-style: solid !important;
outline-offset: 1px !important;
box-shadow: 0 0 0 4px white !important;

@include ms-high-contrast-override {
forced-color-adjust: none;
outline-color: Highlight !important;
box-shadow: 0 0 0 4px HighlightText !important;
}
}
}

.insights-highlight-container .insights-highlight-box .insights-highlight-text {
cursor: default !important;
pointer-events: all !important;
font-size: 1em !important;
padding: 0.3em !important;
float: right !important;
position: relative !important;
text-align: center !important;
.insights-highlight-outline-dashed {
// Non-HC outline-color is intentionally unset; individual formatters should override it
outline-width: 2px !important;
outline-style: dashed !important;
outline-offset: 1px !important;
box-shadow: 0 0 0 4px white !important;

@include ms-high-contrast-override {
forced-color-adjust: none;
outline-color: Highlight !important;
box-shadow: 0 0 0 4px HighlightText !important;
}
}
}

:host {
Expand Down
6 changes: 3 additions & 3 deletions src/injected/visualization/accessible-names-formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export class AccessibleNamesFormatter implements Formatter {
}

public static style: HeadingStyleConfiguration = {
borderColor: '#8D4DFF',
outlineColor: '#8D4DFF',
fontColor: '#FFFFFF',
};

Expand Down Expand Up @@ -48,13 +48,13 @@ export class AccessibleNamesFormatter implements Formatter {
const config: DrawerConfiguration = {
textBoxConfig: {
fontColor: AccessibleNamesFormatter.style.fontColor,
background: AccessibleNamesFormatter.style.borderColor,
background: AccessibleNamesFormatter.style.outlineColor,
text: accessibleNameToDisplay?.accessibleName,
fontWeight: '400',
fontSize: '10px',
outline: '3px dashed',
},
borderColor: AccessibleNamesFormatter.style.borderColor,
outlineColor: AccessibleNamesFormatter.style.outlineColor,
outlineStyle: 'dashed',
outlineWidth: '3px',
showVisualization: true,
Expand Down
2 changes: 1 addition & 1 deletion src/injected/visualization/base-drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export abstract class BaseDrawer implements Drawer {
windowUtils: WindowUtils,
shadowUtils: ShadowUtils,
drawerUtils: DrawerUtils,
formatter: Formatter = null,
formatter: Formatter,
) {
this.dom = dom;
this.containerClass = containerClass;
Expand Down
6 changes: 2 additions & 4 deletions src/injected/visualization/formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ import { DialogRenderer } from '../dialog-renderer';
import { AxeResultsWithFrameLevel } from '../frameCommunicators/html-element-axe-results-helper';

export interface DrawerConfiguration extends SimpleHighlightDrawerConfiguration {
outlineStyle?: string;
outlineWidth?: string;
borderColor: string;
outlineStyle?: 'solid' | 'dashed';
outlineColor?: string;
showVisualization: boolean;
failureBoxConfig?: FailureBoxConfig;
toolTip?: string;
Expand Down Expand Up @@ -38,7 +37,6 @@ export interface BoxConfig {
boxWidth?: string;
fontSize?: string;
fontWeight?: string;
outline?: string;
}

export interface StrokeConfiguration {
Expand Down
12 changes: 6 additions & 6 deletions src/injected/visualization/frame-formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,25 @@ import { FailureInstanceFormatter } from './failure-instance-formatter';
import { DrawerConfiguration } from './formatter';

export interface FrameStyleConfiguration {
borderColor: string;
outlineColor: string;
fontColor: string;
contentText: string;
}

export class FrameFormatter extends FailureInstanceFormatter {
public static frameStyles: { [frameType: string]: FrameStyleConfiguration } = {
frame: {
borderColor: '#0066CC',
outlineColor: '#0066CC',
fontColor: '#FFFFFF',
contentText: 'F',
},
iframe: {
borderColor: '#00CC00',
outlineColor: '#00CC00',
fontColor: '#FFFFFF',
contentText: 'I',
},
default: {
borderColor: '#C00000',
outlineColor: '#C00000',
fontColor: '#FFFFFF',
contentText: '',
},
Expand All @@ -45,9 +45,9 @@ export class FrameFormatter extends FailureInstanceFormatter {
textBoxConfig: {
fontColor: style.fontColor,
text: style.contentText,
background: style.borderColor,
background: style.outlineColor,
},
borderColor: style.borderColor,
outlineColor: style.outlineColor,
outlineStyle: 'solid',
showVisualization: true,
textAlign: 'center',
Expand Down
20 changes: 10 additions & 10 deletions src/injected/visualization/heading-formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { FailureInstanceFormatter } from './failure-instance-formatter';
import { DrawerConfiguration } from './formatter';

export interface HeadingStyleConfiguration {
borderColor: string;
outlineColor: string;
fontColor: string;
}

Expand All @@ -25,31 +25,31 @@ export class HeadingFormatter extends FailureInstanceFormatter {

public static headingStyles: { [level: string]: HeadingStyleConfiguration } = {
'1': {
borderColor: '#0066CC',
outlineColor: '#0066CC',
fontColor: '#FFFFFF',
},
'2': {
borderColor: '#CC0099',
outlineColor: '#CC0099',
fontColor: '#FFFFFF',
},
'3': {
borderColor: '#008000',
outlineColor: '#008000',
fontColor: '#FFFFFF',
},
'4': {
borderColor: '#6600CC',
outlineColor: '#6600CC',
fontColor: '#FFFFFF',
},
'5': {
borderColor: '#008080',
outlineColor: '#008080',
fontColor: '#FFFFFF',
},
'6': {
borderColor: '#996633',
outlineColor: '#996633',
fontColor: '#FFFFFF',
},
blank: {
borderColor: '#C00000',
outlineColor: '#C00000',
fontColor: '#FFFFFF',
},
};
Expand All @@ -72,9 +72,9 @@ export class HeadingFormatter extends FailureInstanceFormatter {
textBoxConfig: {
fontColor: style.fontColor,
text,
background: style.borderColor,
background: style.outlineColor,
},
borderColor: style.borderColor,
outlineColor: style.outlineColor,
outlineStyle: 'solid',
showVisualization: true,
textAlign: 'center',
Expand Down
38 changes: 10 additions & 28 deletions src/injected/visualization/highlight-box-drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,33 +22,19 @@ export class HighlightBoxDrawer extends BaseDrawer {
protected dialogRenderer: DialogRenderer;
private clientUtils: ClientUtils;

public static defaultConfiguration: DrawerConfiguration = {
borderColor: 'rgb(255, 255, 255)',
textBoxConfig: {
fontColor: 'rgb(255, 255, 255)',
background: '#FFFFFF',
text: null,
boxWidth: '2em',
},
outlineStyle: 'solid',
showVisualization: true,
};

constructor(
dom: Document,
containerClass: string,
windowUtils: WindowUtils,
shadowUtils: ShadowUtils,
drawerUtils: DrawerUtils,
clientUtils: ClientUtils,
formatter: Formatter = null,
formatter: Formatter,
private readonly getElementsToHighlight: typeof getTargetElementsFromResult = getTargetElementsFromResult,
) {
super(dom, containerClass, windowUtils, shadowUtils, drawerUtils, formatter);
this.clientUtils = clientUtils;
if (this.formatter) {
this.dialogRenderer = this.formatter.getDialogRenderer();
}
this.dialogRenderer = this.formatter?.getDialogRenderer();
}

public initialize(config: DrawerInitData<HtmlElementAxeResults>): void {
Expand All @@ -74,13 +60,10 @@ export class HighlightBoxDrawer extends BaseDrawer {
const body = currentDom.body;
const bodyStyle = this.windowUtils.getComputedStyle(body);

let drawerConfig = HighlightBoxDrawer.defaultConfiguration;
if (this.formatter) {
drawerConfig = this.formatter.getDrawerConfiguration(
element,
data,
) as DrawerConfiguration;
}
const drawerConfig = this.formatter.getDrawerConfiguration(
element,
data,
) as DrawerConfiguration;

let elementBoundingClientRect: BoundingRect = element.getBoundingClientRect();
if (drawerConfig.getBoundingRect) {
Expand All @@ -106,10 +89,10 @@ export class HighlightBoxDrawer extends BaseDrawer {
}

const wrapper = currentDom.createElement('div');
wrapper.setAttribute('class', 'insights-highlight-box');
wrapper.style.outlineStyle = drawerConfig.outlineStyle;
wrapper.style.outlineColor = drawerConfig.borderColor;
wrapper.style.outlineWidth = drawerConfig.outlineWidth;
wrapper.classList.add('insights-highlight-box');
wrapper.classList.add(`insights-highlight-outline-${drawerConfig.outlineStyle ?? 'solid'}`);
wrapper.style.outlineColor = drawerConfig.outlineColor;

wrapper.style.top = this.drawerUtils.getContainerTopOffset(offset).toString() + 'px';
wrapper.style.left = this.drawerUtils.getContainerLeftOffset(offset).toString() + 'px';
wrapper.style.minWidth =
Expand Down Expand Up @@ -178,7 +161,6 @@ export class HighlightBoxDrawer extends BaseDrawer {
box.style.color = boxConfig.fontColor;
box.style.fontSize = boxConfig.fontSize;
box.style.fontWeight = boxConfig.fontWeight;
box.style.outline = boxConfig.outline;
box.style.setProperty('width', boxConfig.boxWidth, 'important');
box.style.setProperty('cursor', drawerConfig.cursor, 'important');
box.style.setProperty('text-align', drawerConfig.textAlign, 'important');
Expand Down
2 changes: 1 addition & 1 deletion src/injected/visualization/highlight-box-formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class HighlightBoxFormatter extends FailureInstanceFormatter {
): DrawerConfiguration {
const drawerConfig: DrawerConfiguration = {
failureBoxConfig: this.getFailureBoxConfig(data),
borderColor: IssuesFormatter.style.borderColor,
outlineColor: IssuesFormatter.style.outlineColor,
outlineStyle: 'solid',
showVisualization: true,
textAlign: 'center',
Expand Down
Loading

0 comments on commit b34426b

Please sign in to comment.