Skip to content

Comments

Assign IDs to all principles.#455

Merged
jyasskin merged 2 commits intow3ctag:mainfrom
jyasskin:principle-ids
Mar 13, 2025
Merged

Assign IDs to all principles.#455
jyasskin merged 2 commits intow3ctag:mainfrom
jyasskin:principle-ids

Conversation

@jyasskin
Copy link
Collaborator

@jyasskin jyasskin commented Feb 6, 2025

I noticed this when I wanted to link to one of these, and the ID was a couple lines long.


Preview | Diff

@jyasskin jyasskin added the editorial A minor change that is not substantive in nature label Feb 6, 2025
@jyasskin jyasskin requested a review from npdoty February 6, 2025 19:31
index.html Outdated
Comment on lines 68 to 74
for (const practice of document.querySelectorAll(".practicelab")) {
if (!practice.id) {
utils.showWarning(
`Principle should be assigned an id.`, { elements: [practice] }
);
}
}
Copy link
Member

@csarven csarven Mar 13, 2025

Choose a reason for hiding this comment

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

This might also work? #codeshedding =)

Suggested change
for (const practice of document.querySelectorAll(".practicelab")) {
if (!practice.id) {
utils.showWarning(
`Principle should be assigned an id.`, { elements: [practice] }
);
}
}
document.querySelectorAll(".practicelab:not([id])").forEach(practice => {
utils.showWarning(
`Principle should be assigned an id.`, { elements: [practice] }
);
});

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point on the :not([id]). I've generally concluded that language-level loops are more readable than the functional-programming methods, when they're not significantly longer, but I'll switch the selector.

@jyasskin jyasskin merged commit 33bd8f3 into w3ctag:main Mar 13, 2025
1 of 2 checks passed
@jyasskin jyasskin deleted the principle-ids branch March 13, 2025 20:18
github-actions bot added a commit that referenced this pull request Mar 13, 2025
SHA: 33bd8f3
Reason: push, by jyasskin

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

editorial A minor change that is not substantive in nature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants