Skip to content

Commit

Permalink
Factor heading diagnostics (#1505)
Browse files Browse the repository at this point in the history
* Factor previous and next heading diagnostics into other heading

* Update API documentation

* Add changeset

* Update changeset

* Add heading position to diagnostic

* Use `.tee` to store heading levels

* Update .changeset/funny-ducks-approve.md

Co-authored-by: Jean-Yves Moyen <[email protected]>

* Address review comments

* Mark exported type as public

---------

Co-authored-by: Jean-Yves Moyen <[email protected]>
  • Loading branch information
rcj-siteimprove and Jym77 authored Nov 15, 2023
1 parent 9601f1b commit 1921b0d
Show file tree
Hide file tree
Showing 9 changed files with 223 additions and 202 deletions.
5 changes: 5 additions & 0 deletions .changeset/funny-ducks-approve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@siteimprove/alfa-rules": major
---

**Breaking:** Diagnostics `WithPreviousHeading` and `WithNextHeading` have been replaced by `WithOtherHeading`.
3 changes: 1 addition & 2 deletions docs/review/api/alfa-rules.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ export namespace Diagnostic {
import Languages = languages.Languages;
import LabelAndName = labelAndName.LabelAndName;
import RoleAndRequiredAttributes = roleAndRequiredAttributes.RoleAndRequiredAttributes;
import WithPreviousHeading = withPreviousHeading.WithPreviousHeading;
import WithRoleAndName = withRoleAndName.WithRoleAndName;
import SameNames = sameNames.SameNames;
import WithFirstHeading = withFirstHeading.WithFirstHeading;
Expand All @@ -79,13 +78,13 @@ export namespace Diagnostic {
import ColorErrors = colorError.ColorErrors;
import MatchingClasses = matchingClasses.MatchingClasses;
import WithDeclaration = withDeclaration.WithDeclaration;
import WithNextHeading = withNextHeading.WithNextHeading;
import ClippingAncestors = clippingAncestors.ClippingAncestors;
import Contrast = diagnostic.Contrast;
import TextSpacing = diagnostic.TextSpacing;
import WithBadElements = diagnostic.WithBadElements;
import WithRole = diagnostic.WithRole;
import WithAccessibleName = diagnostic.WithAccessibleName;
import WithOtherHeading = diagnostic.WithOtherHeading;
}

declare namespace experimentalRules {
Expand Down
5 changes: 1 addition & 4 deletions packages/alfa-rules/src/common/act/diagnostic.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import * as languages from "../../sia-r109/rule";
import * as labelAndName from "../../sia-r14/rule";
import * as roleAndRequiredAttributes from "../../sia-r16/rule";
import * as withPreviousHeading from "../../sia-r53/rule";
import * as withRoleAndName from "../../sia-r55/rule";
import * as sameNames from "../../sia-r56/rule";
import * as withFirstHeading from "../../sia-r61/rule";
import * as distinguishingStyles from "../../sia-r62/diagnostics";
import * as matchingClasses from "../../sia-r65/diagnostics";
import * as withDeclaration from "../../sia-r75/rule";
import * as withNextHeading from "../../sia-r78/rule";
import * as clippingAncestors from "../../sia-r83/rule";

// R66, R69
Expand All @@ -24,7 +22,6 @@ export namespace Diagnostic {
export import Languages = languages.Languages;
export import LabelAndName = labelAndName.LabelAndName;
export import RoleAndRequiredAttributes = roleAndRequiredAttributes.RoleAndRequiredAttributes;
export import WithPreviousHeading = withPreviousHeading.WithPreviousHeading;
export import WithRoleAndName = withRoleAndName.WithRoleAndName;
export import SameNames = sameNames.SameNames;
export import WithFirstHeading = withFirstHeading.WithFirstHeading;
Expand All @@ -34,11 +31,11 @@ export namespace Diagnostic {
export import ColorErrors = colorError.ColorErrors;
export import MatchingClasses = matchingClasses.MatchingClasses;
export import WithDeclaration = withDeclaration.WithDeclaration;
export import WithNextHeading = withNextHeading.WithNextHeading;
export import ClippingAncestors = clippingAncestors.ClippingAncestors;
export import Contrast = diagnostic.Contrast;
export import TextSpacing = diagnostic.TextSpacing;
export import WithBadElements = diagnostic.WithBadElements;
export import WithRole = diagnostic.WithRole;
export import WithAccessibleName = diagnostic.WithAccessibleName;
export import WithOtherHeading = diagnostic.WithOtherHeading;
}
1 change: 1 addition & 0 deletions packages/alfa-rules/src/common/diagnostic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ export * from "./diagnostic/text-spacing";
export * from "./diagnostic/with-bad-elements";
export * from "./diagnostic/with-role";
export * from "./diagnostic/with-accessible-name";
export * from "./diagnostic/with-other-heading";
138 changes: 138 additions & 0 deletions packages/alfa-rules/src/common/diagnostic/with-other-heading.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
import { Diagnostic } from "@siteimprove/alfa-act";
import { Element } from "@siteimprove/alfa-dom";
import { Hash } from "@siteimprove/alfa-hash";
import { Option } from "@siteimprove/alfa-option";

/**
* @public
*/
export type HeadingPosition = "previous" | "next";

/**
* @public
*/
export class WithOtherHeading extends Diagnostic {
public static of(message: string): Diagnostic;

public static of(
message: string,
otherHeading: Option<Element>,
currentLevel: number,
otherLevel: number,
otherPosition: HeadingPosition,
): WithOtherHeading;

public static of(
message: string,
otherHeading?: Option<Element>,
currentLevel?: number,
otherLevel?: number,
otherPosition?: HeadingPosition,
): Diagnostic {
return otherHeading === undefined ||
currentLevel === undefined ||
otherLevel === undefined ||
otherPosition === undefined
? Diagnostic.of(message)
: new WithOtherHeading(
message,
otherHeading,
currentLevel,
otherLevel,
otherPosition,
);
}

private readonly _otherHeading: Option<Element>;
private readonly _currentLevel: number;
private readonly _otherLevel: number;
private readonly _otherPosition: HeadingPosition;

private constructor(
message: string,
otherHeading: Option<Element>,
currentLevel: number,
otherLevel: number,
otherPosition: HeadingPosition,
) {
super(message);
this._otherHeading = otherHeading;
this._currentLevel = currentLevel;
this._otherLevel = otherLevel;
this._otherPosition = otherPosition;
}

public get otherHeading(): Option<Element> {
return this._otherHeading;
}

public get currentHeadingLevel(): number {
return this._currentLevel;
}

public get otherHeadingLevel(): number {
return this._otherLevel;
}

public get otherPosition(): HeadingPosition {
return this._otherPosition;
}

public equals(value: WithOtherHeading): boolean;

public equals(value: unknown): value is this;

public equals(value: unknown): boolean {
return (
value instanceof WithOtherHeading &&
value._message === this._message &&
value._otherHeading.equals(this._otherHeading) &&
value._currentLevel === this._currentLevel &&
value._otherLevel === this._otherLevel &&
value._otherPosition === this._otherPosition
);
}

public hash(hash: Hash) {
super.hash(hash);
hash.writeNumber(this._currentLevel);
hash.writeNumber(this._otherLevel);
hash.writeString(this._otherPosition);
this._otherHeading.hash(hash);
}

public toJSON(): WithOtherHeading.JSON {
return {
...super.toJSON(),
otherHeading: this._otherHeading.toJSON(),
currentHeadingLevel: this._currentLevel,
otherHeadingLevel: this._otherLevel,
otherPosition: this._otherPosition,
};
}
}

/**
* @public
*/
export namespace WithOtherHeading {
export interface JSON extends Diagnostic.JSON {
otherHeading: Option.JSON<Element>;
currentHeadingLevel: number;
otherHeadingLevel: number;
otherPosition: HeadingPosition;
}

export function isWithOtherHeading(
value: Diagnostic,
): value is WithOtherHeading;

export function isWithOtherHeading(value: unknown): value is WithOtherHeading;

/**@public */
export function isWithOtherHeading(
value: unknown,
): value is WithOtherHeading {
return value instanceof WithOtherHeading;
}
}
140 changes: 62 additions & 78 deletions packages/alfa-rules/src/sia-r53/rule.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
import { Diagnostic, Rule } from "@siteimprove/alfa-act";
import { Rule } from "@siteimprove/alfa-act";
import { DOM } from "@siteimprove/alfa-aria";
import { Element, Node, Query } from "@siteimprove/alfa-dom";
import { Hash } from "@siteimprove/alfa-hash";
import { Predicate } from "@siteimprove/alfa-predicate";
import { Err, Ok } from "@siteimprove/alfa-result";
import { Page } from "@siteimprove/alfa-web";
import { Option } from "@siteimprove/alfa-option";

import { expectation } from "../common/act/expectation";

import { Scope, Stability } from "../tags";
import { WithOtherHeading } from "../common/diagnostic";

const { hasHeadingLevel, hasRole, isIncludedInTheAccessibilityTree } = DOM;
const { and, equals } = Predicate;
const { and, equals, tee } = Predicate;
const { getElementDescendants } = Query;

export default Rule.Atomic.of<Page, Element>({
Expand All @@ -31,97 +32,80 @@ export default Rule.Atomic.of<Page, Element>({
// * The target is in headings by construction of the applicability.
// * The first element of heading is not a target due to the .skip(1)
// * Therefore headings contain at least on element before the target.
const previous = headings.takeUntil(equals(target)).last().getUnsafe();
const previousHeading = headings
.takeUntil(equals(target))
.last()
.getUnsafe();

let previousLevel = -1;
let currentLevel = -1;

return {
1: expectation(
hasHeadingLevel(device, (currentLevel) =>
hasHeadingLevel(
device,
(previousLevel) => previousLevel >= currentLevel - 1,
)(previous),
hasHeadingLevel(
device,
tee(
(currentLevel) =>
hasHeadingLevel(
device,
tee(
(previousLevel) => previousLevel >= currentLevel - 1,
(p) => (previousLevel = p),
),
)(previousHeading),
(c) => (currentLevel = c),
),
)(target),
() => Outcomes.IsStructured(previous),
() => Outcomes.IsNotStructured(previous),
() =>
Outcomes.IsStructured(
previousHeading,
currentLevel,
previousLevel,
),
() =>
Outcomes.IsNotStructured(
previousHeading,
currentLevel,
previousLevel,
),
),
};
},
};
},
});

/**
* @public
*/
export class WithPreviousHeading extends Diagnostic {
public static of(message: string): Diagnostic;

public static of(message: string, previous: Element): WithPreviousHeading;

public static of(message: string, previous?: Element): Diagnostic {
return previous === undefined
? Diagnostic.of(message)
: new WithPreviousHeading(message, previous);
}

private readonly _previous: Element;

constructor(message: string, previous: Element) {
super(message);
this._previous = previous;
}

public get previous(): Element {
return this._previous;
}

equals(value: WithPreviousHeading): value is this;

equals(value: unknown): value is this;

equals(value: unknown): boolean {
return (
value instanceof WithPreviousHeading &&
value._message === this._message &&
value._previous.equals(this._previous)
);
}

public hash(hash: Hash) {
super.hash(hash);
this._previous.hash(hash);
}

toJSON(): WithPreviousHeading.JSON {
return { ...super.toJSON(), previous: this._previous.toJSON() };
}
}

/**
* @public
*/
export namespace WithPreviousHeading {
export interface JSON extends Diagnostic.JSON {
previous: Element.JSON;
}

/**@public */
export function isWithPreviousHeading(
value: unknown,
): value is WithPreviousHeading {
return value instanceof WithPreviousHeading;
}
}

/**
* @public
*/
export namespace Outcomes {
export const IsStructured = (previous: Element) =>
Ok.of(WithPreviousHeading.of(`The heading is correctly ordered`, previous));
export const IsStructured = (
previousHeading: Element,
currentLevel: number,
previousLevel: number,
) =>
Ok.of(
WithOtherHeading.of(
`The heading is correctly ordered`,
Option.of(previousHeading),
currentLevel,
previousLevel,
"previous",
),
);

export const IsNotStructured = (previous: Element) =>
export const IsNotStructured = (
previousHeading: Element,
currentLevel: number,
previousLevel: number,
) =>
Err.of(
WithPreviousHeading.of(`The heading skips one or more levels`, previous),
WithOtherHeading.of(
`The heading skips one or more levels`,
Option.of(previousHeading),
currentLevel,
previousLevel,
"previous",
),
);
}
Loading

0 comments on commit 1921b0d

Please sign in to comment.