Skip to content

Conversation

@hober
Copy link
Member

@hober hober commented Jul 9, 2021

This doesn't contain any normative or functional changes, merely aesthetic ones.

@hober hober requested review from TanviHacks and erik-anderson July 9, 2021 18:30
@hober hober self-assigned this Jul 9, 2021
charter.html Outdated
if (elementWithID instanceof HTMLHeadingElement) {
const headingLevel = parseInt(elementWithID.localName.substring(1), 10);
sectionCounters[headingLevel]++;
for (let i = headingLevel+1; i < 7; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

Consider replacing lines 700 through 706 with:

      sectionCounters.fill(0, headingLevel + 1);
      let sectionNumber = sectionCounters.slice(0, headingLevel + 1).filter(item=> item != 0);

The slice call is actually not necessary but may make the intent more clear to the reader.

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@erik-anderson erik-anderson left a comment

Choose a reason for hiding this comment

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

Looks good. The change suggested in my comment is optional.

Copy link
Contributor

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

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

Any reason we shouldn't merge this? With Erik's tweak, this seems pretty useful to me.

let sectionCounters = [NaN, 0, 0, 0, 0, 0, 0];
document.querySelectorAll("[id]").forEach(function(elementWithID) {
const permalink = document.createElement("a");
permalink.href = "#" + elementWithID.getAttribute("id");
Copy link
Contributor

Choose a reason for hiding this comment

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

I realize that this has been sitting here awhile, but maybe this could be made absolute by starting with document.querySelector("#about+ul li a").href (or a simpler query if you tag that element more directly, as I think we should). Then the permalink is much closer to perma.

@martinthomson martinthomson merged commit 76153fb into main Oct 3, 2024
@martinthomson martinthomson deleted the charter-aesthetics branch October 3, 2024 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants