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

Assign IDs to all principles. #455

Merged
merged 2 commits into from
Mar 13, 2025
Merged

Assign IDs to all principles. #455

merged 2 commits into from
Mar 13, 2025

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