From 99a17f4dd3c801e0240326190510de8526b4a2d6 Mon Sep 17 00:00:00 2001 From: Marcos Caceres Date: Mon, 4 May 2026 10:45:41 +1000 Subject: [PATCH 1/3] feat(core/best-practices): support custom labels via data-label Closes #3900 --- src/core/best-practices.js | 13 +++++++++++-- tests/spec/core/best-practices-spec.js | 24 ++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/core/best-practices.js b/src/core/best-practices.js index ea48d5f2d1..089fcbf4cd 100644 --- a/src/core/best-practices.js +++ b/src/core/best-practices.js @@ -12,6 +12,7 @@ export const name = "core/best-practices"; const localizationStrings = { en: { best_practice: "Best Practice ", + best_practice_summary: "Best Practices Summary", }, ja: { best_practice: "最良実施例 ", @@ -22,6 +23,10 @@ const localizationStrings = { zh: { best_practice: "最佳实践 ", }, + fr: { + best_practice: "Bonne pratique ", + best_practice_summary: "Résumé des bonnes pratiques", + }, }; const l10n = getIntlData(localizationStrings); const lang = defaultLang in localizationStrings ? defaultLang : "en"; @@ -33,8 +38,10 @@ export function run() { const summaryItems = bpSummary ? document.createElement("ul") : null; [...bps].forEach((bp, num) => { const id = addId(bp, "bp"); + const rawLabel = bp.dataset.label || l10n.best_practice; + const label = `${rawLabel.trimEnd()} `; const localizedBpName = html`${l10n.best_practice}${num + 1}${label}${num + 1}`; // Make the summary items, if we have a summary @@ -57,7 +64,9 @@ export function run() { }); if (bps.length) { if (bpSummary) { - bpSummary.appendChild(html`

Best Practices Summary

`); + const summaryLabel = + bpSummary.dataset.label || l10n.best_practice_summary; + bpSummary.appendChild(html`

${summaryLabel}

`); if (summaryItems) bpSummary.appendChild(summaryItems); } } else if (bpSummary) { diff --git a/tests/spec/core/best-practices-spec.js b/tests/spec/core/best-practices-spec.js index 6d4a1fdd77..92b1eec389 100644 --- a/tests/spec/core/best-practices-spec.js +++ b/tests/spec/core/best-practices-spec.js @@ -81,4 +81,28 @@ describe("Core — Best Practices", () => { ); expect(bps.querySelectorAll("ul li")).toHaveSize(3); }); + + it("uses custom labels from data-label attribute", async () => { + const body = ` +
+

Section

+ P1 + P2 +
+
+
+ `; + const ops = { + config: makeBasicConfig(), + body, + }; + const doc = await makeRSDoc(ops); + const summary = doc.getElementById("bp-summary"); + expect(summary).toBeTruthy(); + const listItems = summary.querySelectorAll("li"); + expect(listItems[0].textContent.trim()).toBe("Principle 1: P1"); + expect(listItems[1].textContent.trim()).toBe("Principle 2: P2"); + const heading = summary.querySelector("h3"); + expect(heading.textContent).toContain("Principles Summary"); + }); }); From 4109cdd524175432f8ef0a101445fc220ea78b7a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 13:26:22 +0000 Subject: [PATCH 2/3] fix(core/best-practices): omit lang attr on bdi for custom data-label; add boxed practice test coverage Agent-Logs-Url: https://github.com/speced/respec/sessions/7c04e153-af93-4a0e-bfe4-3dc140064f8b Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com> --- src/core/best-practices.js | 5 ++++- tests/spec/core/best-practices-spec.js | 30 +++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/core/best-practices.js b/src/core/best-practices.js index 089fcbf4cd..5ca49855be 100644 --- a/src/core/best-practices.js +++ b/src/core/best-practices.js @@ -40,8 +40,11 @@ export function run() { const id = addId(bp, "bp"); const rawLabel = bp.dataset.label || l10n.best_practice; const label = `${rawLabel.trimEnd()} `; + const bdi = html`${label}${num + 1}`; + // Only set lang when using the built-in localized label, not a custom one + if (!bp.dataset.label) bdi.lang = lang; const localizedBpName = html`${label}${num + 1}${bdi}`; // Make the summary items, if we have a summary diff --git a/tests/spec/core/best-practices-spec.js b/tests/spec/core/best-practices-spec.js index 92b1eec389..42e88bf417 100644 --- a/tests/spec/core/best-practices-spec.js +++ b/tests/spec/core/best-practices-spec.js @@ -87,7 +87,9 @@ describe("Core — Best Practices", () => {

Section

P1 - P2 +
+

P2

+
@@ -104,5 +106,31 @@ describe("Core — Best Practices", () => { expect(listItems[1].textContent.trim()).toBe("Principle 2: P2"); const heading = summary.querySelector("h3"); expect(heading.textContent).toContain("Principles Summary"); + + // Boxed practice: verify custom label appears in the visible marker + const selfLink = doc.querySelector(".advisement .self-link"); + expect(selfLink.textContent).toBe("Principle 2"); + + // Custom labels must not carry the built-in locale's lang attribute + const bdi = selfLink.querySelector("bdi"); + expect(bdi.hasAttribute("lang")).toBeFalse(); + }); + + it("sets lang attribute on bdi only for built-in localized labels", async () => { + const body = ` +
+

Section

+
+

BP1

+
+
+ `; + const ops = { + config: makeBasicConfig(), + body, + }; + const doc = await makeRSDoc(ops); + const bdi = doc.querySelector(".advisement .self-link bdi"); + expect(bdi.hasAttribute("lang")).toBeTrue(); }); }); From bcd2f827933f289e4493873a119ce3fd0eafffe5 Mon Sep 17 00:00:00 2001 From: Marcos Caceres Date: Tue, 5 May 2026 16:38:58 +1000 Subject: [PATCH 3/3] Address review: don't force lang on custom data-label markers Only set lang attribute on the bdi element when using the built-in l10n label. Custom data-label text inherits lang from its DOM context naturally, so forcing it can produce incorrect language metadata for assistive technology. Also adds test for boxed practice container with custom label. --- src/core/best-practices.js | 6 +++--- tests/spec/core/best-practices-spec.js | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/core/best-practices.js b/src/core/best-practices.js index 5ca49855be..557b8154ea 100644 --- a/src/core/best-practices.js +++ b/src/core/best-practices.js @@ -38,11 +38,11 @@ export function run() { const summaryItems = bpSummary ? document.createElement("ul") : null; [...bps].forEach((bp, num) => { const id = addId(bp, "bp"); - const rawLabel = bp.dataset.label || l10n.best_practice; + const customLabel = bp.dataset.label; + const rawLabel = customLabel || l10n.best_practice; const label = `${rawLabel.trimEnd()} `; const bdi = html`${label}${num + 1}`; - // Only set lang when using the built-in localized label, not a custom one - if (!bp.dataset.label) bdi.lang = lang; + if (!customLabel) bdi.lang = lang; const localizedBpName = html`${bdi}`; diff --git a/tests/spec/core/best-practices-spec.js b/tests/spec/core/best-practices-spec.js index 42e88bf417..6dc31781e7 100644 --- a/tests/spec/core/best-practices-spec.js +++ b/tests/spec/core/best-practices-spec.js @@ -133,4 +133,27 @@ describe("Core — Best Practices", () => { const bdi = doc.querySelector(".advisement .self-link bdi"); expect(bdi.hasAttribute("lang")).toBeTrue(); }); + + it("uses custom labels in boxed best practice containers", async () => { + const body = ` +
+

Section

+
+ Boxed P1 +

Details here.

+
+
+ `; + const ops = { + config: makeBasicConfig(), + body, + }; + const doc = await makeRSDoc(ops); + const container = doc.querySelector(".advisement"); + expect(container).toBeTruthy(); + const marker = container.querySelector(".marker"); + expect(marker.textContent).toContain("Principle 1"); + const bdi = marker.querySelector("bdi"); + expect(bdi.hasAttribute("lang")).toBeFalse(); + }); });