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

Move presentational role conflict resolution down to Role.from #273

Merged
merged 8 commits into from
Jun 23, 2020
Merged

Move presentational role conflict resolution down to Role.from #273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2326251
Move presentational role conflict resolation down to Role.from
Jym77 Jun 22, 2020
effa90e
Fix the code…
Jym77 Jun 22, 2020
6f6f245
Remove unused code
Jym77 Jun 22, 2020
d9b2929
Streamline comments
Jym77 Jun 22, 2020
6fdee0b
Remove useless empty line
Jym77 Jun 22, 2020
87d28df
Clean up
Jym77 Jun 22, 2020
0366d31
Run prettier
Jym77 Jun 22, 2020
8baef8d
Merge branch 'master' into fix-presentational-conflict-resolution
Jym77 Jun 23, 2020
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
13 changes: 5 additions & 8 deletions packages/alfa-aria/src/feature.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,7 @@ export class Feature<N extends string = string> {
}

export namespace Feature {
export type Aspect<T, A extends Array<unknown> = []> = Mapper<
Element,
T,
A
>;
export type Aspect<T, A extends Array<unknown> = []> = Mapper<Element, T, A>;

export interface Status {
readonly obsolete: boolean;
Expand All @@ -71,7 +67,7 @@ export namespace Feature {
/**
* @internal
*/
readonly allowPresentational?: boolean;
readonly allowPresentational: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

I now realise that this has caused the public API to change in a way that makes it impossible to not touch internal features from the outside as this internal property is now required 🙈

}

const features = Cache.empty<Namespace, Cache<string, Feature>>();
Expand Down Expand Up @@ -309,9 +305,10 @@ Feature.register(

Feature.register(
Namespace.HTML,
Feature.of("img", (element, { allowPresentational = true }) =>
Feature.of("img", (element, { allowPresentational }) =>
Option.of(
allowPresentational && element.attribute("alt").some((alt) => alt.value === "")
allowPresentational &&
element.attribute("alt").some((alt) => alt.value === "")
? "presentation"
: "img"
)
Expand Down
121 changes: 39 additions & 82 deletions packages/alfa-aria/src/node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,75 +231,58 @@ export namespace Node {
if (style.computed("visibility").value.value !== "visible") {
accessibleNode = Branched.of(Container.of(node));
} else {
accessibleNode = Role.from(node)
.flatMap((role) => {
// If the element has a presentational role, but is not allowed to
// be presentational, we fall back to its implicit role by not
// considering its explicit role.
if (
role.some(isPresentational) &&
!isAllowedPresentational(node)
) {
return Role.from(node, {
explicit: false,
allowPresentational: false,
});
}

return Branched.of(role);
})
.flatMap<Node>((role) => {
if (role.some(isPresentational)) {
return Branched.of(Container.of(node));
}
accessibleNode = Role.from(node).flatMap<Node>((role) => {
if (role.some(Role.isPresentational)) {
return Branched.of(Container.of(node));
}

let attributes = Map.empty<string, string>();
let attributes = Map.empty<string, string>();

// First pass: Look up implicit attributes on the role.
if (role.isSome()) {
const queue = [role.get()];
// First pass: Look up implicit attributes on the role.
if (role.isSome()) {
const queue = [role.get()];

while (queue.length > 0) {
const role = queue.pop()!;
while (queue.length > 0) {
const role = queue.pop()!;

for (const [name, value] of role.characteristics.implicits) {
attributes = attributes.set(name, value);
}
for (const [name, value] of role.characteristics.implicits) {
attributes = attributes.set(name, value);
}

for (const name of role.characteristics.inherits) {
for (const role of Role.lookup(name)) {
queue.push(role);
}
for (const name of role.characteristics.inherits) {
for (const role of Role.lookup(name)) {
queue.push(role);
}
}
}
}

// Second pass: Look up implicit attributes on the feature mapping.
for (const namespace of node.namespace) {
for (const feature of Feature.lookup(namespace, node.name)) {
attributes = attributes.concat(feature.attributes(node));
}
// Second pass: Look up implicit attributes on the feature mapping.
for (const namespace of node.namespace) {
for (const feature of Feature.lookup(namespace, node.name)) {
attributes = attributes.concat(feature.attributes(node));
}
}

// Third pass: Look up explicit `aria-*` attributes and set the
// ones that are allowed by the role.
for (const attribute of node.attributes) {
if (
attribute.name.startsWith("aria-") &&
role
.orElse(() => Role.lookup("roletype"))
.some((role) =>
role.isAllowed(property("name", equals(attribute.name)))
)
) {
attributes = attributes.set(attribute.name, attribute.value);
}
// Third pass: Look up explicit `aria-*` attributes and set the
// ones that are allowed by the role.
for (const attribute of node.attributes) {
if (
attribute.name.startsWith("aria-") &&
role
.orElse(() => Role.lookup("roletype"))
.some((role) =>
role.isAllowed(property("name", equals(attribute.name)))
)
) {
attributes = attributes.set(attribute.name, attribute.value);
}
}

return getName(node, device).map((name) =>
Element.of(node, role, name, attributes)
);
});
return getName(node, device).map((name) =>
Element.of(node, role, name, attributes)
);
});
}
}

Expand All @@ -323,29 +306,3 @@ export namespace Node {
});
}
}

const isPresentational: Predicate<Role> = property(
"name",
equals("presentation", "none")
);

/**
* Determine if an element is allowed to be presentational.
*
* @see https://w3c.github.io/aria/#conflict_resolution_presentation_none
*/
const isAllowedPresentational: Predicate<dom.Element> = (element) => {
if (element.tabIndex().isSome()) {
return false;
}

return Role.lookup("roletype").some((role) => {
for (const attribute of role.characteristics.supports) {
if (element.attribute(attribute).isSome()) {
return false;
}
}

return true;
});
};
39 changes: 36 additions & 3 deletions packages/alfa-aria/src/role.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,9 @@ export namespace Role {
): Branched<Option<Role>, Browser> {
const role = element.attribute("role").map((attr) => attr.value.trim());

const allowedPresentational =
options.allowPresentational ?? isAllowedPresentational(element);

return (
Branched.of<Option<string>, Browser>(
role.map((role) => role.toLowerCase())
Expand All @@ -251,15 +254,22 @@ export namespace Role {
const role = Role.lookup(name);

if (
// If the role is not abstract...
role.some(
(role) => role.category !== Role.Category.Abstract
) &&
// ...and it's not a presentational role in a forbidden context...
!(
role.some(Role.isPresentational) &&
!isAllowedPresentational
)
) {
// ...then we got ourselves a valid explicit role...
return role;
}
}
}

// ...otherwise, default to implicit role computation.
return None;
})
.orElse(() => {
Expand All @@ -270,7 +280,7 @@ export namespace Role {
return feature.flatMap((feature) =>
feature
.role(element, {
allowPresentational: options.allowPresentational,
allowPresentational: allowedPresentational,
})
.flatMap(Role.lookup)
);
Expand All @@ -284,12 +294,14 @@ export namespace Role {
}

export namespace from {
export interface Options extends Feature.RoleOptions {
export interface Options extends Partial<Feature.RoleOptions> {
readonly explicit?: boolean;
readonly implicit?: boolean;
}
}

export const isPresentational = hasName("presentation", "none");

export function hasName(predicate: Predicate<string>): Predicate<Role>;

export function hasName(
Expand All @@ -313,6 +325,27 @@ export namespace Role {
}
}

/**
* Determine if an element is allowed to be presentational.
*
* @see https://w3c.github.io/aria/#conflict_resolution_presentation_none
*/
const isAllowedPresentational: Predicate<Element> = (element) => {
if (element.tabIndex().isSome()) {
return false;
}

return Role.lookup("roletype").some((role) => {
for (const attribute of role.characteristics.supports) {
if (element.attribute(attribute).isSome()) {
return false;
}
}

return true;
});
};

import "./role/separator";

import "./role/abstract/command";
Expand Down