Skip to content

Commit

Permalink
Use layout to improve handling of interposed elements (#1464)
Browse files Browse the repository at this point in the history
* Add failing test

* Add initial implementation of automatically answering interposed question when we have layout

* Add test of ignored-interposed-elements question behavior when layout is available

* Remove comments

* Add helper function for getting bounding box of element for a specific device

* Extract ignored interposed answer logic to function

* Add changesets

* Fix changeset

* Extract API

* Address review comments

* Refactor for reuse and readability

* Remove redundant test

* Add optional context parameter to

* Add changeset

* Extract API

* Fix typo

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

* Fix typo

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

* Remove redundant comment

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

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Jean-Yves Moyen <[email protected]>
  • Loading branch information
3 people authored Sep 7, 2023
1 parent 10da6ba commit 9f74d99
Show file tree
Hide file tree
Showing 12 changed files with 149 additions and 6 deletions.
7 changes: 7 additions & 0 deletions .changeset/calm-camels-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@siteimprove/alfa-rules": minor
---

**Changed:** Color contrast rules, currently SIA-R66 and SIA-R69, can now tell which interposed elements can be ignored if layout is available.

If layout is not available the rules keep the current behavior of asking a `ignored-interposed-elements` question.
5 changes: 5 additions & 0 deletions .changeset/empty-ears-cheat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@siteimprove/alfa-selector": minor
---

**Added:** A function `isEmpty` to `Context` class
7 changes: 7 additions & 0 deletions .changeset/lovely-bees-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@siteimprove/alfa-style": minor
---

**Added:** A function for getting the bounding box of an element given a device.

This should be the only way of accessing an elements bounding box and prepares us for having device dependent boxes.
2 changes: 2 additions & 0 deletions docs/review/api/alfa-selector.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ export class Context {
// (undocumented)
isActive(element: Element): boolean;
// (undocumented)
isEmpty(): boolean;
// (undocumented)
isFocused(element: Element): boolean;
// (undocumented)
isHovered(element: Element): boolean;
Expand Down
5 changes: 5 additions & 0 deletions docs/review/api/alfa-style.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { Percentage } from '@siteimprove/alfa-css';
import { Position } from '@siteimprove/alfa-css';
import { Predicate } from '@siteimprove/alfa-predicate';
import { Rectangle } from '@siteimprove/alfa-css';
import { Rectangle as Rectangle_2 } from '@siteimprove/alfa-rectangle';
import { Rotate } from '@siteimprove/alfa-css';
import { Serializable } from '@siteimprove/alfa-json';
import { Shadow } from '@siteimprove/alfa-css';
Expand All @@ -48,6 +49,9 @@ import { Tuple } from '@siteimprove/alfa-css';
import { Unit } from '@siteimprove/alfa-css';
import { URL } from '@siteimprove/alfa-css';

// @public
function getBoundingBox(element: Element, device: Device, context?: Context): Option<Rectangle_2>;

// @public (undocumented)
function getOffsetParent(element: Element, device: Device): Option<Element>;

Expand Down Expand Up @@ -440,6 +444,7 @@ export namespace Style {
const // Warning: (ae-forgotten-export) The symbol "element" needs to be exported by the entry point index.d.ts
//
// (undocumented)
getBoundingBox: typeof element.getBoundingBox, // (undocumented)
getOffsetParent: typeof element.getOffsetParent, // (undocumented)
getPositioningParent: typeof element.getPositioningParent, // (undocumented)
hasBorder: typeof element.hasBorder, // (undocumented)
Expand Down
49 changes: 47 additions & 2 deletions packages/alfa-rules/src/common/expectation/contrast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ import { Cache } from "@siteimprove/alfa-cache";
import { RGB } from "@siteimprove/alfa-css";
import { Device } from "@siteimprove/alfa-device";
import { Element, Node, Text } from "@siteimprove/alfa-dom";
import { Set } from "@siteimprove/alfa-set";
import { Iterable } from "@siteimprove/alfa-iterable";
import { None, Option } from "@siteimprove/alfa-option";
import { Set } from "@siteimprove/alfa-set";
import { Style } from "@siteimprove/alfa-style";

import { expectation } from "../act/expectation";
import { Group } from "../act/group";
Expand All @@ -16,6 +18,7 @@ import { isLargeText } from "../predicate";
const { isElement } = Element;
const { flatMap, map, takeWhile } = Iterable;
const { min, max, round } = Math;
const { getBoundingBox } = Style;

/**
* @deprecated This is only used in the deprecated R66v1 and R69v1.
Expand Down Expand Up @@ -113,7 +116,11 @@ export function hasSufficientContrast(
"ignored-interposed-elements",
Group.of(interposedDescendants),
target
).answerIf(interposedDescendants.isEmpty(), []);
).answerIf(
getIntersectors(parent, interposedDescendants, device).map((intersectors) =>
interposedDescendants.subtract(intersectors)
)
);

const foregrounds = Question.of("foreground-colors", target);
const backgrounds = Question.of("background-colors", target);
Expand Down Expand Up @@ -223,3 +230,41 @@ export function contrast(foreground: RGB, background: RGB): number {

return round(contrast * 100) / 100;
}

/**
* Finds elements from a collection of candidate that intersect with a given element
*
* @remarks
* If either the element or one of the `candidates` doesn't have layout, we can't fully decide intersection and return `None`.
*/
function getIntersectors(
element: Element<string>,
candidates: Iterable<Element>,
device: Device
): Option<Iterable<Element>> {
// If the collection of candidates is empty we don't need layout to determine that there are no intersectors
if (Iterable.isEmpty(candidates)) {
return Option.of(candidates);
}

const elementBox = getBoundingBox(element, device);

if (
!elementBox.isSome() ||
Iterable.some(candidates, (candidate) =>
getBoundingBox(candidate, device).isNone()
)
) {
return None;
}

return Option.of(
Iterable.filter(
candidates,
(canditate) =>
elementBox
.get()
.intersects(getBoundingBox(canditate, device).getUnsafe()) // Presence of the box is guaranteed by the above check
)
);
}
48 changes: 44 additions & 4 deletions packages/alfa-rules/test/sia-r69/rule.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@ import { Future } from "@siteimprove/alfa-future";
import { None } from "@siteimprove/alfa-option";
import { test } from "@siteimprove/alfa-test";

import { RGB, Percentage, Keyword } from "@siteimprove/alfa-css";
import { Keyword, Percentage, RGB } from "@siteimprove/alfa-css";
import { Device } from "@siteimprove/alfa-device";
import { Style } from "@siteimprove/alfa-style";

import R69 from "../../src/sia-r69/rule";
import { Contrast as Diagnostic } from "../../src/common/diagnostic/contrast";
import { Contrast as Outcomes } from "../../src/common/outcome/contrast";
import R69 from "../../src/sia-r69/rule";

import { evaluate } from "../common/evaluate";
import { passed, failed, cantTell, inapplicable } from "../common/outcome";
import { cantTell, failed, inapplicable, passed } from "../common/outcome";

import { oracle } from "../common/oracle";
import { ColorError, ColorErrors } from "../../src/common/dom/get-colors";
import { oracle } from "../common/oracle";

const rgb = (r: number, g: number, b: number, a: number = 1) =>
RGB.of(
Expand Down Expand Up @@ -856,3 +856,43 @@ test("evaluate() can tell when encountering an opaque background before an absol
}),
]);
});

test("evaluate() can tell when interposed descendant overlaps offset parent, but does not overlap target", async (t) => {
const target = h.text("Hello World");

const document = h.document([
<body box={{ x: 8, y: 8, width: 1070, height: 126 }}>
<div
style={{
position: "absolute",
backgroundColor: "green",
opacity: "50%",
top: "100px",
left: "100px",
width: "100px",
height: "100px",
}}
box={{ x: 100, y: 100, width: 100, height: 100 }}
></div>
<div box={{ x: 8, y: 8, width: 1070, height: 18 }}>{target}</div>
<br />
<br />
<br />
<br />
<br />
<br />
</body>,
]);

t.deepEqual(await evaluate(R69, { document }), [
passed(R69, target, {
1: Outcomes.HasSufficientContrast(21, 4.5, [
Diagnostic.Pairing.of(
["foreground", rgb(0, 0, 0)],
["background", rgb(1, 1, 1)],
21
),
]),
}),
]);
});
4 changes: 4 additions & 0 deletions packages/alfa-selector/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ export class Context {
this._state = state;
}

public isEmpty(): boolean {
return this._state.isEmpty();
}

public hasState(element: Element, state: Context.State): boolean {
return this._state.get(element).some((found) => (found & state) !== 0);
}
Expand Down
1 change: 1 addition & 0 deletions packages/alfa-style/src/element/element.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from "./helpers/get-bounding-box";
export * from "./helpers/get-offset-parent";
export * from "./helpers/get-positioning-parent";
export * from "./predicate/has-border";
Expand Down
25 changes: 25 additions & 0 deletions packages/alfa-style/src/element/helpers/get-bounding-box.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { Device } from "@siteimprove/alfa-device";
import { Element } from "@siteimprove/alfa-dom";
import { None, Option } from "@siteimprove/alfa-option";
import { Rectangle } from "@siteimprove/alfa-rectangle";
import { Context } from "@siteimprove/alfa-selector";

/**
* @public
* Gets the bounding box, corresponding to a specific device, of an element
*
* @privateRemarks
* We don't use the passed in device yet, but later we should use it to ensure the device used to collect the bounding box corresponds to the current device
*/
export function getBoundingBox(
element: Element,
device: Device,
context: Context = Context.empty()
): Option<Rectangle> {
// We assume layout is only grabbed on empty contexts, so if the context is non-empty we don't have layout
if (!context.isEmpty()) {
return None;
}

return element.box;
}
1 change: 1 addition & 0 deletions packages/alfa-style/src/style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ export namespace Style {
export type Inherited<N extends Name> = Longhands.Inherited<N>;

export const {
getBoundingBox,
getOffsetParent,
getPositioningParent,
hasBorder,
Expand Down
1 change: 1 addition & 0 deletions packages/alfa-style/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
},
"files": [
"src/element/element.ts",
"src/element/helpers/get-bounding-box.ts",
"src/element/helpers/get-offset-parent.ts",
"src/element/helpers/get-positioning-parent.ts",
"src/element/predicate/has-border.ts",
Expand Down

0 comments on commit 9f74d99

Please sign in to comment.