Skip to content
This repository has been archived by the owner on Oct 28, 2023. It is now read-only.

preamble and blame tweaks #12

Closed
wants to merge 1 commit into from
Closed

Conversation

scoskey
Copy link
Collaborator

@scoskey scoskey commented Jan 11, 2018

create a preamble tag, move blame code so both name and blame can be used

Copy link
Owner

@pkra pkra left a comment

Choose a reason for hiding this comment

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

Just a small comment.

@@ -123,6 +123,13 @@ for (let name of names) {
renamedNode.classList.add('name');
}

// convert blames to spans
Copy link
Owner

Choose a reason for hiding this comment

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

Just to check this is intentional: this will move the blame-span out of the heading, causing a break between heading and blame. I'm guessing you want to change the the theorem headings later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, all I know is it looked good.

@@ -67,6 +67,12 @@ licensing.classList.add('license');
licensing.innerHTML = 'Derived from <a href="' + articleMeta.source + '">' + articleMeta.source + '</a>, ' + articleMeta.license + ' and licensed as such.'
articleInfo.appendChild(licensing);

// preamble

const preamble = document.querySelector('preamble');
Copy link
Owner

Choose a reason for hiding this comment

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

Nice!

@pkra
Copy link
Owner

pkra commented Jan 12, 2018

Very Cool! A question is in my review.

@scoskey
Copy link
Collaborator Author

scoskey commented Jan 12, 2018

Likely unnecessary after current rewrite.

@scoskey scoskey closed this Jan 12, 2018
@pkra
Copy link
Owner

pkra commented Jan 12, 2018

I just totally broke this PR.. I'll integrate the changes.

@pkra pkra mentioned this pull request Jan 12, 2018
pkra added a commit that referenced this pull request Jan 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants