Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Make failure highlight more visible #6164

Merged
merged 9 commits into from
Nov 22, 2022
Merged

fix: Make failure highlight more visible #6164

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c3d9e7c
fix: Make failure highlight more visible
segamiken Nov 3, 2022
a64b221
Refactor formatter configs to specify outlineColor instead of borderC…
dbjorge Nov 16, 2022
f83f385
remove obsolete type that referred to borderColor
dbjorge Nov 16, 2022
ab39f63
Update highlight box drawing to use box-shadow + outline
dbjorge Nov 16, 2022
a212fa6
Update e2e snapshots
dbjorge Nov 17, 2022
7d6a7ed
Revert incorrect snapshot update
dbjorge Nov 17, 2022
8146c4e
Fix drawing-controller tests by re-enabling mock HighlightBoxDrawers
dbjorge Nov 17, 2022
28bf1f9
Fix new box-shadow behavior not applying to dashed outlines in HC mode
dbjorge Nov 17, 2022
795f8bd
Update solid outline style to be 2px wide per PR feedback
dbjorge Nov 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/assessments/headings/headings-instance-details-column-renderer.tsx
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/assessments/landmarks/landmarks-instance-details-column-renderer.tsx
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/assessments/page/frametitle-instance-details-column-renderer.tsx
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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: 1px !important;
outline-style: solid !important;
outline-offset: 1px !important;
box-shadow: 0 0 0 3px white !important;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opinion: I find 1px outline over 3px shadow makes the red outline not stand out as much as I'm used to, especially around images.
image
I think 2px outline over 4px shadow is a little clearer:
image
Will totally defer to @ferBonnin here.

Copy link
Contributor

@ferBonnin ferBonnin Nov 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean @peterdur specially for white backgrounds; I agree that 2px makes it more clearer but it also uses more space. I don't feel strongly about one way or another so if we prefer it, lets go with 2px with 4px shadow 🙂


@include ms-high-contrast-override {
forced-color-adjust: none;
outline-color: Highlight !important;
box-shadow: 0 0 0 3px 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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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