-
-
Notifications
You must be signed in to change notification settings - Fork 92
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
feat(cover-page): add support for subtitle and revision infos #431
base: main
Are you sure you want to change the base?
Conversation
lib/document/document-converter.js
Outdated
const docTitle = node.getDocument().getDocumentTitle({ partition: true }) | ||
//If the document has a subtitle, we split title in h1 / subtitle in h2 | ||
if (docTitle.hasSubtitle()){ | ||
return `<div id="cover" class="title-page front-matter"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid duplicating the outer HTML <div>
, you can do:
const titlePageContent = /* ... */
// ...
return `<div id="cover" class="title-page front-matter">${titlePageContent}</div>`
lib/document/document-converter.js
Outdated
<div id="doc-author">${node.getDocument().getAuthor()}</div> | ||
<div id="doc-rev-infos">Version ${node.getDocument().getRevisionNumber()}, ${node.getDocument().getRevisionDate()}</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the HTML structure should be:
<div class="details">
<span class="author">${node.getDocument().getAuthor()}</span>
<span class="revnumber"><!-- ... --></span>
<span class="revremark"><!-- ... --></span>
</div>
For the revision date, we could use <time>
with datetime
(if the date is valid).
Also keep in mind that an AsciiDoc document can have more than on author.
We should probably use getAuthors
and display author's email if available.
For inspiration, you can take a look at:
lib/document/document-converter.js
Outdated
return `<div id="cover" class="title-page front-matter"> | ||
<h1>${docTitle.getCombined()}</h1> | ||
<div id="doc-author">${node.getDocument().getAuthor()}</div> | ||
<div id="doc-rev-infos">Version ${node.getDocument().getRevisionNumber()}, ${node.getDocument().getRevisionDate()}</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code should not be duplicated, you can construct partial HTML blocks:
const title = docTitle.hasSubtitle() ? `<h1>...</h1><h2>...</h2>` : `<h1>...</h1>`
const detail = `<div class="details">...</div>`
return `<div id="cover" class="title-page front-matter">
${title}
${detail}
</div>`
Hey @Nexyll Thanks for this pull request.
That's a good idea 👍
I don't think we should use ids for styling but classes (see comments).
That's true but this project is still in alpha stage so we can break things (especially when it's for the better!). |
ea87a8e
to
8e61780
Compare
Hey, Thanks for your comments. I've just pushed a new version with your suggestions 😉 |
8e61780
to
850a90b
Compare
It actually brings quite a few questions. Let's take an example:
And here's the result using Asciidoctor PDF 1.5.4:
And here's the result using Asciidoctor 2.0.12
I think it would make sense to produce the same HTML whether title page is enabled or not. In order to support subtitle, I think we should use the following HTML structure: <header>
<h1>Document Title</h1>
<p class="subtitle">Subtitle</p>
</header> Also, when there's more than one author, we could use an unordered list: <ul class="authors">
<li><span class="author">Guillaume Grossetie</span><a class="email" href="mailto:[email protected]">[email protected]</a></li>
<li><span class="author">Kismet Lee</span></li>
</ul> |
Since I'm not 100% sure what we should do, I need to give it more thoughts and experiment a bit. I checked out your branch, I will try my suggestion to find its limits. |
@Nexyll Just to keep you informed, I'm now leading the effort to create a modern HTML converter upstream: https://github.com/asciidoctor/asciidoctor/projects/1 Following our discussion, I wrote a complete proposal on which semantic constructs we should use for the author(s) and revision: asciidoctor/asciidoctor#3967 |
Thanks! If I understand correctly, rather than generating something on the asciidoctor-web-pdf side, the goal is to achieve the front page generation cleanly in the new html rendering system of asciidoctor ? If this is the case, I don't know how I can help, but if you have an idea don't hesitate to ask me. |
Yes, that's correct.
You can review pull requests and comment on issues related to this project: https://github.com/asciidoctor/asciidoctor/projects/1 Currently, I'm working on the revision: asciidoctor/asciidoctor#4022 And here's the new converter: https://github.com/asciidoctor/asciidoctor/blob/feature/html-converter-next/lib/asciidoctor/converter/semantic-html5.rb You can also take a look at: https://github.com/asciidoctor/asciidoctor/tree/feature/html-converter-next/test/fixtures/semantic-html5-scenarios author-with-email.adoc = Document Title
Stuart Rackham <founder@asciidoc.org> will produce: author-with-email.html <header>
<p class="byline">
<span class="author">Stuart Rackham <a href="mailto:[email protected]" rel="author">[email protected]</a></span>
</p>
</header> |
A first step to implement #84.
The idea beind this MR is to bring the default generation of asciidoctor-pdf-web closer to what is already done by asciidoctor-pdf.
For other properties (author, revisions, ...) they can be added in divs with specific ids for styling, according to what is already done in this MR.
The drawback of the changes is that existing documents may have based their stylesheet on the fact that the author was in an h2 tag on the cover page. This MR introduce breacking changes for those documents.