Update SIA R67 #272
Update SIA R67 #272
Conversation
| const emptyAlt = getById("empty-alt"); | ||
| const roleNone = getById("role-none"); | ||
| const rolePresentation = getById("role-presentation"); | ||
| const svg = getById("svg"); |
There was a problem hiding this comment.
This can be done using destructuring and without additional helpers and need for IDs:
const [img, video, object, svg] = document
.first()
.get()
.children()
.filter(Element.isElement);There was a problem hiding this comment.
True, but this is less resilient to changes in the HTML.
Additionally, ids are commenting (in the HTML) what this target is testing, and can be handy for debugging messages from inside the rule itself.
There was a problem hiding this comment.
That's reasonable 👍 What we need then is a unified approach to this rather than ad-hoc helpers scattered throughout rule spec files. Test code is most annoying code to maintain so it needs to be super maintainable 🙈
There was a problem hiding this comment.
Yes, I totally agree. If we decide that IDs + getter is the way we wanna go, we should of course make the helper common and update existing tests to use it…
I've also been considering writing a wrapper for Document.of+Element.fromElement that we seem to use pretty often in tests…
There was a problem hiding this comment.
#281 should hopefully help on the last point.
There was a problem hiding this comment.
Yeah it gets a bit better. Still need to
Document.fromDocument(
h.document([
<html>
<head>
<title>Hello world</title>
</head>
</html>
])
);instead of
documentFromHtml(<html>
<head>
<title>Hello world</title>
</head>
</html>);
| @@ -261,7 +261,7 @@ export namespace Role { | |||
| // ...and it's not a presentational role in a forbidden context... | |||
| !( | |||
| role.some(Role.isPresentational) && | |||
| !isAllowedPresentational | |||
| !allowedPresentational | |||
| const emptyAlt = getById("empty-alt"); | ||
| const roleNone = getById("role-none"); | ||
| const rolePresentation = getById("role-presentation"); | ||
| const svg = getById("svg"); |
There was a problem hiding this comment.
Yes, I totally agree. If we decide that IDs + getter is the way we wanna go, we should of course make the helper common and update existing tests to use it…
I've also been considering writing a wrapper for Document.of+Element.fromElement that we seem to use pretty often in tests…
Jym77
dismissed
kasperisager’s stale review
Fixed a nasty bug + updated to latest Sanshika
kasperisager
added
the
patch
* master: Update SIA R67 (#272)
* master: (21 commits) Rework the way DOM parent pointers are handled (#283) Create a GitHub release as part of release workflow Backtrack on shortcut links v0.3.0 Prepare release Update changelog Update changelog `RoleOptions#allowPresentational` must be optional Update SIA R67 (#272) Update lockfile Correctly resolve important and overridden CSS properties (#282) Introduce HyperScript-like DSL for constructing DOM (#281) Use dynamic import rather than `require()` for loading formatters (#278) Add `Future#get()` and accept thunked promises in `Future.from()` (#279) Fix default argument type parameter of several types Speedup table building (#276) Add initial support for `background` shorthand property (#277) Output stack in CLI errors if available Chesterton's fence strikes again! Correctly pull `.message` from expectations ...
|
Just a heads up, @singingknight, that this is not ready for the platform yet. |
Update following
https://github.com/Siteimprove/sanshikan/pull/105