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.

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

feat(core/dfn): add data-defines on some patterns of term/definition association #4620

Merged
merged 10 commits into from
Jan 29, 2024
33 changes: 33 additions & 0 deletions src/core/dfn.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,4 +237,37 @@ function addContractDefaults() {
for (const dfn of exportableDfns) {
dfn.dataset.export = "";
}

// - Sets data-defines on well-known definition content patterns
marcoscaceres marked this conversation as resolved.
Show resolved Hide resolved

// A dt with class dfn-desc (on in a dl with such a class)
dontcallmedom marked this conversation as resolved.
Show resolved Hide resolved
// containing a definition
// indicates that the following dd or div element contains its prose content
/** @type NodeListOf<HTMLElement> */
const describedDTs = document.querySelectorAll(
"dl.dfn-desc dt:has(dfn[data-dfn-type]), dt.dfn-desc:has(dfn[data-dfn-type])"
dontcallmedom marked this conversation as resolved.
Show resolved Hide resolved
);
for (const dt of describedDTs) {
const dfnId = dt.querySelector("dfn[id]").id;
dontcallmedom marked this conversation as resolved.
Show resolved Hide resolved

const dfnContent = /** @type {HTMLElement | null} */ (
dt.nextElementSibling
);
if (dfnContent && dfnId && ["DIV", "DD"].includes(dfnContent.tagName)) {
dfnContent.dataset.defines = `#${dfnId}`;
}
}

// a non-dt element with class dfn-desc containing a definition
// indicates that the said element contains its prose content
/** @type NodeListOf<HTMLElement> */
const otherDescriptionContainers = document.querySelectorAll(
":not(dt):not(dl).dfn-desc:has(dfn[data-dfn-type])"
tidoust marked this conversation as resolved.
Show resolved Hide resolved
);
for (const el of otherDescriptionContainers) {
const dfnId = el.querySelector("dfn[id]").id;
dontcallmedom marked this conversation as resolved.
Show resolved Hide resolved
if (dfnId) {
el.dataset.defines = `#${dfnId}`;
}
}
}
27 changes: 27 additions & 0 deletions tests/spec/core/dfn-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -655,5 +655,32 @@ describe("Core — Definitions", () => {
expect(errors[0].message).toContain("Declares both");
expect(errors[1].message).toContain("but also has a");
});

it("assigns data-defines on well-known pattern", async () => {
const body = `
<section>
<h2>Definition and its description</h2>
<p id="desc1" class="dfn-desc">
A <dfn>definition</dfn> can also have an associated description
</p>
<dl class="dfn-desc">
<dt><dfn>different convention</dfn></dt>
<dd id="desc2">Different conventions can be applied to associate a term with its description</dd>
</dl>
<dl>
Copy link
Member

Choose a reason for hiding this comment

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

Thinking out loud:
Do we want/need to handle the cases with multiple dt or multiple dd?
Multiple dt: as an alias.. sounds like job for data-lt
Multiple dd: why?

<dt class="dfn-desc"><dfn>local convention</dfn></dt>
<dd id="desc3">The local convention can be applied to a dt individually</dd>
</dl>
</section>
`;
const ops = makeStandardOps(null, body);
const doc = await makeRSDoc(ops);
const desc1 = doc.getElementById("desc1");
const desc2 = doc.getElementById("desc2");
const desc3 = doc.getElementById("desc3");
expect(desc1.dataset.defines).toBe("#dfn-definition");
expect(desc2.dataset.defines).toBe("#dfn-different-convention");
expect(desc3.dataset.defines).toBe("#dfn-local-convention");
});
});
});