PreTeXtBook / pretext

PreTeXt: an authoring and publishing system for scholarly documents
https://pretextbook.org
Other
254 stars 203 forks source link

Poem margins #994

Closed davidfarmer closed 5 years ago

davidfarmer commented 5 years ago

Line 1995 of the current mathbook-html.xsl sets the top (and bottom) margin of a poem to be 0. This does not look good (see Section 24 of sample article, the start of the "Charge of..." poem).

And even if it did look good, that should be set in the css.

I can address this, if you wish.

rbeezer commented 5 years ago

poem is the biggest offender (I think) of using hard-coded style attributes. I'd really like to get the style totally over to CSS. And it is a root cause of two exceptions in the middle of some very general code, which have gotten under my skin.

I already have an existing commit to leave classes in place on poems, but drop the hard-coded styles. I could make a "beta" of the humanities demo, and send you the commit so you could copy the CSS as a starting point. You could address this exterior spacing as part of that?

(There are some small alignment bits that might need some attention once we get most of it free.)

Would that plan work?

davidfarmer commented 5 years ago

Yes, I can do the CSS for poems if you make a beta HTML.

Classes I will style include:

.poem .poemtitle (not sure why that is a class, instead of .title, since you can select on .title within .poem) .stanza .poemauthorleft (I assume there also is a .poemauthorright . Does that get the appropriate dash in front of it? Will that dash be added by xslt or CSS?) .poemlineleft

No need to add to this list here: I'll look at the humanities demo.

On Wed, 6 Feb 2019, Rob Beezer wrote:

poem is the biggest offender (I think) of using hard-coded style attributes. I'd really like to get the style totally over to CSS. And it is a root cause of two exceptions in the middle of some very general code, which have gotten under my skin.

I already have an existing commit to leave classes in place on poems, but drop the hard-coded styles. I could make a "beta" of the humanities demo, and send you the commit so you could copy the CSS as a starting point. You could address this exterior spacing as part of that?

(There are some small alignment bits that might need some attention once we get most of it free.)

Would that plan work?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.[AAM6LM3sMkwd1A_2hbAIwj06PiEyxqDsks5vK5n7gaJpZM4amf6c.gif]

rbeezer commented 5 years ago

OK, as much @style removed as I could. Humanities sample at:

https://pretextbook.org/beta/2019-02-07-poem-css/hia.html

The current version is unchanged at the website, for visual conparisons.

Here is the current diff. Some is housekeeping, but you can see the @style that have been removed.

diff --git a/xsl/mathbook-html.xsl b/xsl/mathbook-html.xsl
index 76328ad..9c65ff4 100644
--- a/xsl/mathbook-html.xsl
+++ b/xsl/mathbook-html.xsl
@@ -1870,7 +1870,7 @@ along with MathBook XML.  If not, see <http://www.gnu.org/licenses/>.
 <!-- this.  Likely replace use of this template        -->
 <!-- by the template "heading-title" above             -->
 <xsl:template match="poem" mode="heading-poem">
-    <div class="poemtitle" style="font-weight: bold; text-align: center; font-size: 120%">
+    <div class="poemtitle">
         <xsl:apply-templates select="." mode="title-full"/>
     </div>
 </xsl:template>
@@ -1988,13 +1988,6 @@ along with MathBook XML.  If not, see <http://www.gnu.org/licenses/>.
                 <xsl:attribute name="id">
                     <xsl:apply-templates select="." mode="perm-id" />
                 </xsl:attribute>
-                <!-- this horrible hack should go away once better CSS is in place -->
-                <!-- likely this particular version never gets used                -->
-                <xsl:if test="self::poem">
-                    <xsl:attribute name="style">
-                        <xsl:text>display: table; width: auto; max-width: 90%; margin: 0 auto;</xsl:text>
-                    </xsl:attribute>
-                </xsl:if>
                 <xsl:apply-templates select="." mode="hidden-knowl-link">
                     <xsl:with-param name="placement" select="$placement"/>
                 </xsl:apply-templates>
@@ -2030,13 +2023,6 @@ along with MathBook XML.  If not, see <http://www.gnu.org/licenses/>.
                 <xsl:attribute name="class">
                     <xsl:apply-templates select="." mode="body-css-class" />
                 </xsl:attribute>
-                <!-- this horrible hack should go away once better CSS is in place -->
-                <!-- likely this particular version never gets used                -->
-                <xsl:if test="self::poem">
-                    <xsl:attribute name="style">
-                        <xsl:text>display: table; width: auto; max-width: 90%; margin: 0 auto;</xsl:text>
-                    </xsl:attribute>
-                </xsl:if>
                 <xsl:apply-templates select="." mode="duplicate-hidden-knowl-link" />
             </xsl:element>
         </xsl:when>
@@ -2388,26 +2374,19 @@ along with MathBook XML.  If not, see <http://www.gnu.org/licenses/>.
 <!-- https://github.com/BooksHTML/mathbook-assets/issues/65 -->

 <xsl:template match="poem/author">
-    <xsl:variable name="alignment">
-        <xsl:apply-templates select="." mode="poem-halign"/>
-    </xsl:variable>
-    <xsl:element name="div">
+    <div>
         <xsl:attribute name="class">
             <xsl:text>poemauthor</xsl:text>
-            <xsl:value-of select="$alignment" />
-        </xsl:attribute>
-        <xsl:attribute name="style">
-            <xsl:text>font-style: italic; padding-bottom: 20px; text-align: </xsl:text>
-            <xsl:value-of select="$alignment" />
+            <xsl:apply-templates select="." mode="poem-halign"/>
         </xsl:attribute>
         <xsl:apply-templates/>
-    </xsl:element>
+    </div>
 </xsl:template>

 <xsl:template match="stanza">
-    <div class="stanza" style="padding-bottom: 20px">
+    <div class="stanza">
         <xsl:if test="title">
-            <div class="stanzatitle" style="font-weight: bold; text-align: center">
+            <div class="stanzatitle">
                 <xsl:apply-templates select="." mode="title-full"/>
             </div>
         </xsl:if>
@@ -2422,29 +2401,26 @@ along with MathBook XML.  If not, see <http://www.gnu.org/licenses/>.
     <xsl:variable name="indentation">
         <xsl:apply-templates select="." mode="poem-indent"/>
     </xsl:variable>
-    <xsl:element name="div">
+    <div>
         <xsl:attribute name="class">
             <xsl:text>poemline</xsl:text>
-            <xsl:value-of select="$alignment" />
+            <xsl:apply-templates select="." mode="poem-halign"/>
         </xsl:attribute>
-        <xsl:attribute name="style">
-            <!-- Hanging indentation for overly long lines -->
-            <xsl:text>margin-left: 4em; text-indent: -4em; </xsl:text>
-            <xsl:text>text-align: </xsl:text>
-            <xsl:value-of select="$alignment" />
-        </xsl:attribute>
-        <xsl:if test="$alignment='left'"><!-- Left Alignment: Indent from Left -->
+        <!-- Left Alignment: Indent from Left -->
+        <xsl:if test="$alignment='left'">
             <xsl:call-template name="poem-line-indenting">
-                <xsl:with-param name="count"><xsl:value-of select="$indentation"/></xsl:with-param>
+                <xsl:with-param name="count" select="$indentation"/>
             </xsl:call-template>
         </xsl:if>
-        <xsl:apply-templates/><!-- Center Alignment: Ignore Indentation -->
-        <xsl:if test="$alignment='right'"><!-- Right Alignment: Indent from Right -->
+        <!-- Center Alignment: Ignore Indentation -->
+        <xsl:apply-templates/>
+        <!-- Right Alignment: Indent from Right -->
+        <xsl:if test="$alignment='right'">
             <xsl:call-template name="poem-line-indenting">
-                <xsl:with-param name="count"><xsl:value-of select="$indentation"/></xsl:with-param>
+                <xsl:with-param name="count" select="$indentation"/>
             </xsl:call-template>
         </xsl:if>
-    </xsl:element>
+    </div>
 </xsl:template>

 <xsl:template name="poem-line-indenting">
@@ -2452,7 +2428,7 @@ along with MathBook XML.  If not, see <http://www.gnu.org/licenses/>.
     <xsl:choose>
         <xsl:when test="(0 >= $count)"/>
         <xsl:otherwise>
-            <span class="tab" style="margin-left: 2em"></span>
+            <span class="tab"/>
             <xsl:call-template name="poem-line-indenting">
                 <xsl:with-param name="count" select="$count - 1"/>
             </xsl:call-template>
@@ -5256,7 +5232,7 @@ along with MathBook XML.  If not, see <http://www.gnu.org/licenses/>.
 <!-- duplicate the title, which is handled specially   -->
 <!-- max-width is at 100%, not 90%                     -->
 <xsl:template match="poem" mode="panel-html-box">
-    <div class="poem" style="display: table; width: auto; max-width: 100%; margin: 0 auto;">
+    <div class="poem">
         <xsl:apply-templates select="stanza"/>
         <xsl:apply-templates select="author"/>
     </div>
davidfarmer commented 5 years ago

Once some more style has been removed, and some css classes have been changed, this can be merged after my ToC refactor is merged.

Note: your diff says that the hard-coded styles were removed from .poem, but I still see it in the sample html.

Please change class="authorleft" to class="author left", i.e., add a space, and similarly for center and right.

Change poemtitle to title and same for stanzatitle.

With those changes, and after the other pull request, the poetry document should look right. (If I missed something, I should be able to fix quickly.)

davidfarmer commented 5 years ago

Sorry, I skipped a step.

Change class="poemauthorleft" to class="author left" , and similarly for right and center.

That is, it is just "author" not "poemauthor", and the author and alignment parts of the class are separated.

This lets the CSS style "author" in general, and then separately only treat the left/right/center .

And we select on .poem .author, so no need to specify .poemauthor .

rbeezer commented 5 years ago

I missed the third "horrible hack" with a @style - thanks for catching that!

Made the other suggested changes - looks good on humanities sample. Merged, but not yet pushed.

Got distracted by the titles. Two suggestions/proposals. These would:

Now:

div.poem
  div.title

Proposal:

article.poem
  h6.heading
    span.title

Now:

div.stanza
  div.title

Proposal:

div.stanza
  h6.heading
    span.title

Would this be straight-forward enough? If so, I can make a beta.

davidfarmer commented 5 years ago

Seems fine.

I'll adjust the CSS if you post a beta.

On Mon, 11 Feb 2019, Rob Beezer wrote:

I missed the third "horrible hack" with a @style - thanks for catching that!

Made the other suggested changes - looks good on humanities sample. Merged, but not yet pushed.

Got distracted by the titles. Two suggestions/proposals. These would:

  • place h6, which will eventually change so every page uniformly goes h1 to h6 for better accessibility.
  • use a template that gets used lots of other places, thus being more reliable and maintainable.
  • look more like other titled pieces (so better conversion to Braille, etc)

Now:

div.poem div.title

Proposal:

article.poem h6.heading span.title

Now:

div.stanza div.title

Proposal:

div.stanza h6.heading span.title

Would this be straight-forward enough? If so, I can make a beta.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.[AAM6LA90a1LX7zyBMhOmSkf85xBqZAqsks5vMcGJgaJpZM4amf6c.gif]

rbeezer commented 5 years ago

Excellent. XSL ready on a branch. Beta at:

https://pretextbook.org/beta/2019-02-11-poem-titles/hia.html

davidfarmer commented 5 years ago

Should be okay, but do a quick reload.

If you wanted to change .poemlineleft to .lineleft, and similar, then I can adjust the css. (The selectors assume .poem as an ancestor.)

On Mon, 11 Feb 2019, Rob Beezer wrote:

Excellent. XSL ready on a branch. Beta at:

https://pretextbook.org/beta/2019-02-11-poem-titles/hia.html

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.[AAM6LIqykI-mOD2d112xRgtwlNlU2r-oks5vMcwvgaJpZM4amf6c.gif]

rbeezer commented 5 years ago

On 2/11/19 12:35 PM, David W. Farmer wrote:

Should be okay, but do a quick reload.

Stanza titles are smaller, but I think that is an improvmeent.

If you wanted to change .poemlineleft to .lineleft, and similar, then I can adjust the css. (The selectors assume .poem as an ancestor.)

Beta in a minute.

rbeezer commented 5 years ago

Beta now at the same URL as above.

davidfarmer commented 5 years ago

Adjusted the selector, and made the subtitles slightly bigger.

What do you think of .line.left instead of .lineleft ?

That is, class="line left" (with a space) instead of "lineleft" ?

Am I trying to perfect the markup too much?

On Mon, 11 Feb 2019, Rob Beezer wrote:

On 2/11/19 12:35 PM, David W. Farmer wrote:

Should be okay, but do a quick reload.

Stanza titles are smaller, but I think that is an improvmeent.

If you wanted to change .poemlineleft to .lineleft, and similar, then I can adjust the css. (The selectors assume .poem as an ancestor.)

Beta in a minute.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.[AAM6LGhuv1Xmshd6icxxZNlQ67Yde5jlks5vMdZlgaJpZM4amf6c.gif]

rbeezer commented 5 years ago

On 2/11/19 12:56 PM, David W. Farmer wrote:

Adjusted the selector, and made the subtitles slightly bigger.

Great.

What do you think of .line.left instead of .lineleft ?

That is, class="line left" (with a space) instead of "lineleft" ?

Had the same thought. Yes, I'd say.

Am I trying to perfect the markup too much?

I am regularly pleased that we do things carefully and changes are easy. ;-) We've come this far, no reason to slack off now!

I'll make a beta.

rbeezer commented 5 years ago

Beta up now, w/out knowls.

On 2/11/19 1:00 PM, Rob Beezer wrote:

On 2/11/19 12:56 PM, David W. Farmer wrote:

Adjusted the selector, and made the subtitles slightly bigger.

Great.

What do you think of .line.left instead of .lineleft ?

That is, class="line left" (with a space) instead of "lineleft" ?

Had the same thought. Yes, I'd say.

Am I trying to perfect the markup too much?

I am regularly pleased that we do things carefully and changes are easy. ;-) We've come this far, no reason to slack off now!

I'll make a beta.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/issues/994#issuecomment-462492432, or mute the thread https://github.com/notifications/unsubscribe-auth/ABy2csAPKj9Egdwde8KuTGueliDY8KWaks5vMdoJgaJpZM4amf6c.

davidfarmer commented 5 years ago

Done. Can't think of anything else. Ready for you to push

On Mon, 11 Feb 2019, Rob Beezer wrote:

Beta up now, w/out knowls.

On 2/11/19 1:00 PM, Rob Beezer wrote:

On 2/11/19 12:56 PM, David W. Farmer wrote:

Adjusted the selector, and made the subtitles slightly bigger.

Great.

What do you think of .line.left instead of .lineleft ?

That is, class="line left" (with a space) instead of "lineleft" ?

Had the same thought. Yes, I'd say.

Am I trying to perfect the markup too much?

I am regularly pleased that we do things carefully and changes are easy. ;-) We've come this far, no reason to slack off now!

I'll make a beta.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/issues/994#issuecomment-462492432, or mute the thread https://github.com/notifications/unsubscribe-auth/ABy2csAPKj9Egdwde8KuTGueliDY8KWaks5vMdoJgaJpZM4amf6c.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.[AAM6LOv_XyrUE-lpZCxY1XvZG9bY4spvks5vMdqPgaJpZM4amf6c.gif]

rbeezer commented 5 years ago

Pushed, and rebuilding website now.

Changes at: 00b72d9813639dc0cf1357b2e94681e1b84be097 and 23a164861de69320d4f4860713852e473feac511

Thanks - I'm glad to get this one cleaned up. HTML + CSS + JS is all looking very nice.

davidfarmer commented 5 years ago

What styling in the source should we eliminate next?

On Mon, 11 Feb 2019, Rob Beezer wrote:

Pushed, and rebuilding website now.

Changes at: 00b72d9 and 23a1648

Thanks - I'm glad to get this one cleaned up. HTML + CSS + JS is all looking very nice.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.[AAM6LMVp6V1Rkf9_25cd6rfE-GqorzKYks5vMfRPgaJpZM4amf6c.gif]

rbeezer commented 5 years ago

The "notation list" is a table where three tr/th have style="text-align:left;" and tr/td with style="text-align:left; vertical-align:top;".

Videos have some sloppy links (buttons?) with style=

LaTeX macros are defined in:

<div class="hidden-content" style="display:none">

Perhaps the style is not even needed?

Images have some extra style, but I'll have to review that carefully. "Fixing" hN right now. Confirm you never use the hN for styling? I'm free to make them run continguously "down" a page, no matter how chunking is set?