Browse Source

fix: misaligned section markers (#4006)

pull/4016/head
Marcos Cáceres 8 months ago committed by GitHub
parent
commit
784b0f0e62
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 12
      src/core/id-headers.js
  2. 23
      src/core/linter-rules/no-headingless-sections.js
  3. 2
      src/styles/mdn-annotation.css.js
  4. 42
      src/styles/respec.css.js
  5. 2
      src/w3c/linter-rules/required-sections.js
  6. 2
      tests/spec/core/biblio-spec.js
  7. 8
      tests/spec/core/data-include-spec.js
  8. 12
      tests/spec/core/dfn-index-spec.js
  9. 8
      tests/spec/core/id-headers-spec.js
  10. 8
      tests/spec/core/idl-index-spec.js
  11. 10
      tests/spec/core/issues-notes-spec.js
  12. 47
      tests/spec/core/linter-rules/no-headingless-sections-spec.js
  13. 16
      tests/spec/core/markdown-spec.js
  14. 9
      tests/spec/core/sections-spec.js
  15. 7
      tests/spec/w3c/abstract-spec.js
  16. 8
      tests/spec/w3c/headers-spec.js

12
src/core/id-headers.js

@ -43,9 +43,13 @@ export function run(conf) {
h.closest(".appendix") ? "Appendix" : "Section",
h.querySelector(":scope > bdi.secno")
);
h.insertAdjacentElement(
"afterend",
html`<a href="${`#${id}`}" class="self-link" aria-label="${label}"></a>`
);
const wrapper = html`<div class="header-wrapper"></div>`;
h.replaceWith(wrapper);
const selfLink = html`<a
href="#${id}"
class="self-link"
aria-label="${label}"
></a>`;
wrapper.append(h, selfLink);
}
}

23
src/core/linter-rules/no-headingless-sections.js

@ -26,22 +26,19 @@ const localizationStrings = {
};
const l10n = getIntlData(localizationStrings);
const hasNoHeading = ({ firstElementChild: elem }) => {
return elem === null || /^h[1-6]$/.test(elem.localName) === false;
};
export function run(conf) {
if (!conf.lint?.[ruleName]) {
return;
}
const offendingElements = [...document.querySelectorAll("section")].filter(
hasNoHeading
const sections = Array.from(document.getElementsByTagName("section"));
const offendingElements = sections.filter(
({ firstElementChild: e }) => !e || !e.matches(".header-wrapper")
);
if (offendingElements.length) {
showWarning(l10n.msg, name, {
hint: l10n.hint,
elements: offendingElements,
});
}
if (!offendingElements.length) return;
showWarning(l10n.msg, name, {
hint: l10n.hint,
elements: offendingElements,
});
}

2
src/styles/mdn-annotation.css.js

@ -8,7 +8,7 @@ export default css`
position: absolute;
right: 0.3em;
min-width: 0;
margin-top: 3em;
margin-top: 3rem;
}
.mdn details {

42
src/styles/respec.css.js

@ -208,7 +208,7 @@ details.respec-tests-details > li {
padding-left: 1em;
}
a[href].self-link:hover {
.self-link:hover {
opacity: 1;
text-decoration: none;
background-color: transparent;
@ -218,34 +218,44 @@ aside.example .marker > a.self-link {
color: inherit;
}
:is(h2, h3, h4, h5, h6) + a.self-link {
border: none;
.header-wrapper {
display: flex;
align-items: center;
}
:is(h2, h3, h4, h5, h6):not(#toc h2) {
position: relative;
left: -.5em;
}
:is(h2, h3, h4, h5, h6):not(#toc h2) + a.self-link {
color: inherit;
order: -1;
position: relative;
top: -2.75em;
left: -1.4em;
margin-bottom: -2em;
display: block;
font-size: 100%;
left: -1.1em;
top: .8rem;
font-size: 1rem;
opacity: 0.5;
}
:is(h2, h3, h4, h5, h6) + a.self-link::before {
content: "§";
opacity: 0.5;
text-decoration: none;
line-height: 1.2em;
vertical-align: middle;
color: var(--heading-text);
}
:is(h4, h5, h6) + a.self-link::before {
color: black;
}
:is(h4, h5, h6) + a.self-link {
top: 0.6rem;
}
@media (max-width: 767px) {
dd {
margin-left: 0;
}
/* Don't position self-link in headings off-screen */
:is(h2, h3, h4, h5, h6) + a.self-link {
left: 101%;
}
}
@media print {

2
src/w3c/linter-rules/required-sections.js

@ -78,7 +78,7 @@ export function run(conf) {
for (const header of headers) {
const clone = header.cloneNode(true);
// section number and self-link anchor
clone.querySelectorAll("bdi, .self-link")?.forEach(elem => elem.remove());
clone.querySelectorAll("bdi")?.forEach(elem => elem.remove());
const text = norm(clone.textContent);
if (missingRequiredSections.has(text)) {
missingRequiredSections.delete(text);

2
tests/spec/core/biblio-spec.js

@ -302,7 +302,7 @@ it("allows custom content in the references section", async () => {
`,
};
const doc = await makeRSDoc(ops);
const { textContent: h2Text } = doc.querySelector("#references > h2");
const { textContent: h2Text } = doc.querySelector("#references h2");
const { textContent: pText } = doc.querySelector("#references > p");
const [normRef, infoRef] = doc.querySelectorAll("#references h3");
expect(doc.documentElement.lang).toBe("nl");

8
tests/spec/core/data-include-spec.js

@ -46,7 +46,9 @@ describe("Core — Data Include", () => {
expect(missing).toBeNull();
const included = doc.getElementById("replacement-test");
expect(included).toBeTruthy();
const heading = doc.querySelector("#replacement-test > h3");
const heading = doc.querySelector(
"#replacement-test > div.header-wrapper > h3"
);
expect(heading).toBeTruthy();
expect(heading.textContent).toBe("Replacement");
});
@ -84,7 +86,7 @@ describe("Core — Data Include", () => {
};
ops.config.format = "markdown";
const doc = await makeRSDoc(ops);
const h2 = doc.querySelector("#includes > h2");
const h2 = doc.querySelector("#includes > div.header-wrapper > h2");
expect(h2).toBeTruthy();
expect(h2.textContent).toBe("1. PASS");
expect(doc.querySelectorAll("*[data-include]")).toHaveSize(0);
@ -205,7 +207,7 @@ describe("Core — Data Include", () => {
const ops = makeStandardOps(null, body);
const doc = await makeRSDoc(ops);
const h2 = doc.querySelector("#includes > h2");
const h2 = doc.querySelector("#includes > div.header-wrapper > h2");
expect(h2).toBeTruthy();
expect(h2.textContent).toContain("Test");

12
tests/spec/core/dfn-index-spec.js

@ -16,10 +16,10 @@ describe("Core — dfn-index", () => {
expect(index.querySelectorAll("h2")).toHaveSize(1);
expect(index.querySelector("h2").textContent).toBe("A. Index");
expect(index.firstElementChild).toBe(index.querySelector("h2"));
expect(index.querySelector("h2 + a + p")).toEqual(
doc.getElementById("pass")
expect(index.firstElementChild.firstElementChild).toBe(
index.querySelector("h2")
);
expect(index.querySelector("div + p")).toEqual(doc.getElementById("pass"));
});
it("doesn't override existing content in section#index", async () => {
@ -33,7 +33,7 @@ describe("Core — dfn-index", () => {
expect(index.querySelectorAll("h2")).toHaveSize(1);
expect(index.querySelector("h2").textContent).toBe("A. el índex");
expect(index.firstElementChild).toBe(
expect(index.firstElementChild.firstElementChild).toBe(
index.querySelector("h2#custom-heading")
);
expect(index.querySelector("p#custom-paragraph").textContent).toBe("PASS");
@ -47,7 +47,7 @@ describe("Core — dfn-index", () => {
"Terms defined by this specification"
);
expect(
localIndex.querySelector("h3 + a.self-link + ul.index")
localIndex.querySelector("div.header-wrapper + ul.index")
).toBeTruthy();
const externalIndexHeading = externalIndex.querySelector("h3");
@ -55,7 +55,7 @@ describe("Core — dfn-index", () => {
"Terms defined by reference"
);
expect(
externalIndex.querySelector("h3 + a.self-link + ul.index")
externalIndex.querySelector("div.header-wrapper + ul.index")
).toBeTruthy();
});

8
tests/spec/core/id-headers-spec.js

@ -79,10 +79,14 @@ describe("Core - ID headers", () => {
});
it("adds section links", () => {
const test1 = doc.querySelector("#test-1 > h2 + a.self-link");
const test1 = doc.querySelector(
"#test-1 > div.header-wrapper > h2 + a.self-link"
);
expect(test1.getAttribute("href")).toBe("#test-1");
const test2 = doc.querySelector("#pass > h2 + a.self-link");
const test2 = doc.querySelector(
"#pass > div.header-wrapper > h2 + a.self-link"
);
expect(test2.getAttribute("href")).toBe("#custom-id");
});

8
tests/spec/core/idl-index-spec.js

@ -52,7 +52,7 @@ interface Bar {
pre.querySelectorAll(".idlHeader").forEach(elem => elem.remove());
expect(pre.textContent).toBe(expectedIDL);
const header = doc.querySelector("#idl-index > h2");
const header = doc.querySelector("#idl-index > div.header-wrapper > h2");
expect(header).not.toBeNull();
expect(header.textContent).toBe("1. IDL Index");
});
@ -292,10 +292,12 @@ dictionary Bar {
const idlIndex = doc.getElementById("idl-index");
expect(idlIndex).not.toBeNull();
expect(idlIndex.querySelector("pre")).toBeNull();
const header = doc.querySelector("#idl-index > h2");
const header = doc.querySelector("#idl-index > div.header-wrapper > h2");
expect(header).not.toBeNull();
expect(header.textContent).toBe("1. PASS");
expect(doc.querySelectorAll("#idl-index > h2")).toHaveSize(1);
expect(
doc.querySelectorAll("#idl-index > div.header-wrapper > h2")
).toHaveSize(1);
});
it("doesn't include ids in the cloned indexed", async () => {

10
tests/spec/core/issues-notes-spec.js

@ -366,7 +366,9 @@ describe("Core — Issues and Notes", () => {
`,
};
const doc = await makeRSDoc(ops);
const { textContent } = doc.querySelector("#issue-summary > h2");
const { textContent } = doc.querySelector(
"#issue-summary > div.header-wrapper > h2"
);
expect(doc.documentElement.lang).toBe("es");
expect(textContent).toContain("Resumen de la cuestión");
});
@ -385,7 +387,7 @@ describe("Core — Issues and Notes", () => {
`,
};
const doc = await makeRSDoc(ops);
const h2 = doc.querySelector("#issue-summary > h2");
const h2 = doc.querySelector("#issue-summary > div.header-wrapper > h2");
expect(h2.innerText).toContain("Open Issues");
const p = doc.querySelector("#issue-summary p");
expect(p.innerText).toContain("Here you will find all open issues");
@ -408,11 +410,11 @@ describe("Core — Issues and Notes", () => {
`,
};
const doc = await makeRSDoc(ops);
const h2 = doc.querySelector("#issue-summary > h2");
const h2 = doc.querySelector("#issue-summary > div.header-wrapper > h2");
expect(h2.innerText).toContain("Issue summary");
const p = doc.querySelector("#issue-summary p");
expect(p.innerText).toContain("Here you will find all issues summary");
const div = doc.querySelector("#issue-summary div");
const div = doc.querySelector("#issue-summary div:last-child");
expect(div.innerText).toContain("This is a note");
// Headings other than top level heading should not be detected as issue summary heading
const h3 = doc.querySelector("#issue-summary section h3");

47
tests/unit/core/linter-rules/no-headingless-sections-spec.js → tests/spec/core/linter-rules/no-headingless-sections-spec.js

@ -1,21 +1,22 @@
"use strict";
import { makePluginDoc } from "../../SpecHelper.js";
import {
makeRSDoc,
makeStandardOps,
warningFilters,
} from "../../../spec/SpecHelper.js";
describe("Core Linter Rule - 'no-headingless-sections'", () => {
const modules = [`/src/core/linter-rules/no-headingless-sections.js`];
const warningsFilter = warningFilters.filter(
"core/linter-rules/no-headingless-sections"
);
const config = { lint: { "no-headingless-sections": true } };
async function getWarnings(body) {
const config = { lint: { "no-headingless-sections": true } };
const doc = await makePluginDoc(modules, { config, body });
return doc.respec.warnings.filter(
warning => warning.plugin === "core/linter-rules/no-headingless-sections"
);
}
it("returns error when heading is missing section", async () => {
it("warns when heading is missing section", async () => {
const body = `<section id="test"></section>`;
const warnings = await getWarnings(body);
const opts = makeStandardOps(config, body);
const doc = await makeRSDoc(opts);
const warnings = warningsFilter(doc);
expect(warnings).toHaveSize(1);
const [warning] = warnings;
@ -27,17 +28,19 @@ describe("Core Linter Rule - 'no-headingless-sections'", () => {
it("doesn't complain when sections do have a heading", async () => {
const body = `
<section>
<h2>test</h2>
<section>
<h2>test</h2>
<section>
<h3>Test</h3>
</section>
<section>
<h3>Test</h3>
</section>
<h3>Test</h3>
</section>
<section>
<h3>Test</h3>
</section>
</section>
`;
const warnings = await getWarnings(body);
const opts = makeStandardOps(config, body);
const doc = await makeRSDoc(opts);
const warnings = warningsFilter(doc);
expect(warnings).toHaveSize(0);
});
@ -53,7 +56,9 @@ describe("Core Linter Rule - 'no-headingless-sections'", () => {
</section>
</section>
`;
const warnings = await getWarnings(body);
const opts = makeStandardOps(config, body);
const doc = await makeRSDoc(opts);
const warnings = warningsFilter(doc);
expect(warnings).toHaveSize(1);
expect(warnings[0].elements).toHaveSize(1);
expect(warnings[0].elements[0].id).toBe("badone");

16
tests/spec/core/markdown-spec.js

@ -166,27 +166,27 @@ describe("Core - Markdown", () => {
const doc = await makeRSDoc(ops);
const foo = doc.querySelector("#foo h2");
expect(foo.textContent).toBe("1. Foo");
expect(foo.parentElement.localName).toBe("section");
expect(foo.parentElement.parentElement.localName).toBe("section");
const bar = doc.querySelector("#bar h3");
expect(bar.textContent).toBe("1.1 Bar");
expect(bar.parentElement.localName).toBe("section");
expect(bar.parentElement.parentElement.localName).toBe("section");
const baz = doc.querySelector("#baz h3");
expect(baz.textContent).toBe("1.2 Baz");
expect(baz.parentElement.localName).toBe("section");
expect(baz.parentElement.parentElement.localName).toBe("section");
const foobar = doc.querySelector("#foobar h4");
expect(foobar.textContent).toBe("1.2.1 Foobar");
expect(foobar.parentElement.localName).toBe("section");
expect(foobar.parentElement.parentElement.localName).toBe("section");
const foobaz = doc.querySelector("#foobaz h5");
expect(foobaz.textContent).toBe("1.2.1.1 Foobaz");
expect(foobaz.parentElement.localName).toBe("section");
expect(foobaz.parentElement.parentElement.localName).toBe("section");
const zing = doc.querySelector("#zing h3");
expect(zing.textContent).toBe("1.3 Zing");
expect(zing.parentElement.localName).toBe("section");
expect(zing.parentElement.parentElement.localName).toBe("section");
});
it("gracefully handles jumps in nested headers", async () => {
@ -625,7 +625,9 @@ function getAnswer() {
const ops = makeStandardOps({ format: "markdown" }, body);
ops.abstract = null;
const doc = await makeRSDoc(ops);
const headings = doc.querySelectorAll("body > section > h2");
const headings = doc.querySelectorAll(
"body > section > div.header-wrapper > h2"
);
const headingTitles = [
"Abstract",
"Status of This Document",

9
tests/spec/core/sections-spec.js

@ -18,7 +18,7 @@ describe("Core — sections", () => {
for (let i = 2; i <= 6; i++) {
const context = `h${i}`;
const h = doc.getElementById(context);
const section = h.parentElement;
const section = h.parentElement.parentElement;
expect(section.localName).withContext(context).toBe("section");
expect(section.id).withContext(context).toBe(`section-${i}`);
}
@ -42,7 +42,7 @@ describe("Core — sections", () => {
const doc = await makeRSDoc(ops);
for (let i = 2; i <= 6; i++) {
const h = doc.getElementById(`h${i}`);
const section = h.parentElement;
const section = h.parentElement.parentElement;
expect(section.localName).toBe("section");
expect(section.id).toBe(`section-${i}`);
}
@ -64,7 +64,10 @@ describe("Core — sections", () => {
const ops = makeStandardOps(null, body);
const doc = await makeRSDoc(ops);
for (let i = 2; i <= 6; i++) {
const p = doc.querySelector(`#h${i} + a + p`);
const p = doc
.querySelector(`#h${i}`)
.closest("section")
.querySelector("p");
expect(p.textContent).toBe(`paragraph ${i}`);
const comment = p.nextSibling;
expect(comment.nodeType).toBe(Node.COMMENT_NODE);

7
tests/spec/w3c/abstract-spec.js

@ -72,9 +72,10 @@ describe("W3C — Abstract", () => {
const doc = await makeRSDoc(null, "spec/w3c/abstract-legacy-div.html");
expect(doc.querySelector("div#abstract")).toBeNull();
expect(doc.querySelector("section#abstract.introductory")).toBeTruthy();
expect(doc.querySelector("section#abstract>h2").textContent).toBe(
"Abstract"
);
expect(
doc.querySelector("section#abstract > div.header-wrapper > h2")
.textContent
).toBe("Abstract");
expect(doc.querySelector("section#abstract>p").textContent).toBe(
"The abstract."
);

8
tests/spec/w3c/headers-spec.js

@ -1969,9 +1969,11 @@ describe("W3C — Headers", () => {
const doc = await makeRSDoc(ops);
const sotd = doc.getElementById("sotd");
expect(sotd).toBeTruthy();
expect(sotd.firstElementChild.localName).toBe("h2");
expect(sotd.firstElementChild.textContent).toBe("Override");
expect(sotd.children.length).toBe(2);
const h2 = sotd.querySelector("div.header-wrapper > h2");
expect(h2.textContent).toBe("Override");
// the div that contains the header
expect(sotd.children.length).toBe(1);
expect(sotd.firstElementChild.localName).toBe("div");
expect(sotd.querySelector("a.self-link")).toBeTruthy();
});

Loading…
Cancel
Save