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

Poem margins #994

Closed
davidfarmer opened this issue Feb 7, 2019 · 18 comments
Closed

Poem margins #994

davidfarmer opened this issue Feb 7, 2019 · 18 comments

Comments

@davidfarmer
Copy link
Contributor

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
Copy link
Collaborator

rbeezer commented Feb 7, 2019

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
Copy link
Contributor Author

davidfarmer commented Feb 7, 2019 via email

@rbeezer
Copy link
Collaborator

rbeezer commented Feb 8, 2019

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Collaborator

rbeezer commented Feb 11, 2019

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.

@davidfarmer
Copy link
Contributor Author

davidfarmer commented Feb 11, 2019 via email

@rbeezer
Copy link
Collaborator

rbeezer commented Feb 11, 2019

Excellent. XSL ready on a branch. Beta at:

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

@davidfarmer
Copy link
Contributor Author

davidfarmer commented Feb 11, 2019 via email

@rbeezer
Copy link
Collaborator

rbeezer commented Feb 11, 2019 via email

@rbeezer
Copy link
Collaborator

rbeezer commented Feb 11, 2019

Beta now at the same URL as above.

@davidfarmer
Copy link
Contributor Author

davidfarmer commented Feb 11, 2019 via email

@rbeezer
Copy link
Collaborator

rbeezer commented Feb 11, 2019 via email

@rbeezer
Copy link
Collaborator

rbeezer commented Feb 11, 2019 via email

@davidfarmer
Copy link
Contributor Author

davidfarmer commented Feb 11, 2019 via email

@rbeezer
Copy link
Collaborator

rbeezer commented Feb 11, 2019

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.

@rbeezer rbeezer closed this as completed Feb 11, 2019
@davidfarmer
Copy link
Contributor Author

davidfarmer commented Feb 11, 2019 via email

@rbeezer
Copy link
Collaborator

rbeezer commented Feb 11, 2019

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?

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

No branches or pull requests

2 participants