PreTeXtBook / pretext

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

Add paragraph cells in tables #354

Closed Alex-Jordan closed 7 years ago

Alex-Jordan commented 7 years ago

Here it is for LaTeX and HTML. (Upon reflection today, I don't think it's necessary to pursue this with PG. All cells in WeBWorK tables are paragraph-cells anyway.)

As this is right now, there needs to be a CSS class "justify" with text:align:justify;. David, please add that if this PR is approved.

Comments and alteration requests welcome. My git fu (jugitsu?) can handle changes easily enough now.

rbeezer commented 7 years ago

In order of the diff:

Generally, I think you are working too hard to infer missing widths, which is yielding some overly-complicated XSL and what looks like to me some potential for bugs. Maybe it was my suggestion to condition on "p", but maybe it would be much cleaner to require author to provide a @width to signal a paragraph column. Error-check obvious bad things (sum of given widths over 100%), scream if a cell has a p or a justify without a @width. Then the 1/N device can go, along with a lot of code, like "width-percentage-to-real".

I think I prefer to convert width percentages to reals on the fly in LaTeX (see div 100 for images, sidebyside). A -common template to sanity check (normalize, % last) would be welcome, I think, if that is about all it did. I've decided to pass widths around as strings with % on them to avoid silly errors, and then they often just dump straight into HTML (but not here with pixel device, so you'd need a div 100 here too).

I can fetch changes as a new commit on this branch if you want to work that way. I don't think dev has anything too radical on it ahead of this. I'll rebase and squish when we are done reviewing.

Thanks for this - I know how much work this has been. "Believe me."

Alex-Jordan commented 7 years ago

OK, I'll work on it and just keep this same branch.

Are you intent on capping width at 100%? I can see occasional need to go past that. It would be a big lie anyway because paragraph columns can go alongside non-paragraph columns that have no @width, and just grow to their natural width. And as noted in a thread somewhere earlier today, there is no good way to ascertain their width ahead of time. (How wide is an <m>, etc.?)

I'll clean up all the demo formatting from the units table and make a separate torture test table. CDATA cells, line cells, p cells. colspan in the first two types.

I think @width as a signal for a paragraph column has problems, because within that column you may want non line breaking cells too. But conditioning on p to make it a paragraph cell and throwing an error if the col has no @width is doable. Is that along the lines you were thinking?

I find "$the-cell/ancestor::tabular/child::col" easier to follow, because of the syntax consistency. But I think you are right that it could just be "$the-cell/ancestor::tabular/col"

What if the first row has some colspan cells?

I'd prefer to just count cols, and this was in there in case cols had been omitted. But if a @width is required in a col for a paragraph cell, then it becomes a nonissue.

Can we really normalize if the table is not necessarily 100% wide? Or maybe you mean normalize when the sum of widths surpasses 100%? Part of the difficulty is still that there can be non-paragraph columns. See comment above about how wide is an <m>, etc.

I meant $the-cell/*|$the-cell/text(). Didn't understand that $the-cell/node() captured more than that.

Yes, I noticed width-percentage-to-real disappeared when I did a rebase and there was an unusually large conflict I had to address. Quoting you: 'There is a new modal template "width-percent-to-real" that will save you some bother.' Maybe that's when I decided to drop some footnotes in a table ;)

rbeezer commented 7 years ago

Are you intent on capping width at 100%? I can see occasional need to go past that. It would be a big lie anyway because paragraph columns can go alongside non-paragraph columns that have no @width, and just grow to their natural width. And as noted in a thread somewhere earlier today, there is no good way to ascertain their width ahead of time. (How wide is an , etc.?)

I just meant if all @width exceeded 100%, then scream. Author makes a typo: width="30%, width="700%". Let's catch that. Not perfect, never wrong.

I think @width as a signal for a paragraph column has problems, because within that column you may want non line breaking cells too. But conditioning on p to make it a paragraph cell and throwing an error if the col has no @width is doable. Is that along the lines you were thinking?

Sure. The automatic widths just seemed too difficult and imperfect, so hitting a p cell and screaming about no @width is fine. I think it is worth forcing author to realize that a "p" column has an ambiguous width.

I find "$the-cell/ancestor::tabular/child::col" easier to follow, because of the syntax consistency. But I think you are right that it could just be "$the-cell/ancestor::tabular/col"

Your call. Style.

What if the first row has some colspan cells?

I'd prefer to just count cols, and this was in there in case cols had been omitted. But if a @width is required in a col for a paragraph cell, then it becomes a nonissue.

Right. If you want to count columns, you need to sum colspan. This is there someplace.

Can we really normalize if the table is not necessarily 100% wide? Or maybe you mean normalize when the sum of widths surpasses 100%? Part of the difficulty is still that there can be non-paragraph columns. See comment above about how wide is an , etc.

I just meant normalize(@width). Especially, strip trailing spaces so % really is at the end.

I meant $the-cell/*|$the-cell/text(). Didn't understand that $the-cell/node() captured more than that.

Took me a long time to realize that. Lenz' book is very helpful. Why is the change necessary?

Yes, I noticed width-percentage-to-real disappeared when I did a rebase and there was an unusually large conflict I had to address. Quoting you: 'There is a new modal template "width-percent-to-real" that will save you some bother.' Maybe that's when I decided to drop some footnotes in a table ;)

Sorry about that. Code evolves. ;-) Did you plan to provide support for footnotes in tables? If not, they can just go in a paragaph below the table.

Alex-Jordan commented 7 years ago

Any thoughts on making the width default to something (say 30%) if it is not specified?

In parallel, if there is no width for the col, should this just give a warning, or an error?

rbeezer commented 7 years ago

For sidebyside I went 1/N if no widths were given at all. I'd go that far if

(a) no @width anywhere

(b) and colspan are taken into account to get the correct number of columns.

But I think my preference is to just require a @width for any column with a "p" in it. I'd be inclined to fail (terminate="yes") - author education.

On 08/03/2016 06:37 PM, Alex Jordan wrote:

Any thoughts on making the width default to something (say 30%) if it is not specified?

In parallel, if there is no width for the col, should this just give a warning, or an error?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/pull/354#issuecomment-237426628, or mute the thread https://github.com/notifications/unsubscribe-auth/ABy2cit1qtOwg4ghwAeZGXqDQJ4GCiVsks5qcUJTgaJpZM4JZTdI.

rbeezer commented 7 years ago

For a paragraph column in a tabular in a sidebyside panel, the @width should interpreted as a fraction of the panel's @width. So you'll need to adjust the template for a tabular in a sidebyside to pass a (restricted) with into the general tabular template, so the correct pixel count is used.

On 08/03/2016 06:37 PM, Alex Jordan wrote:

Any thoughts on making the width default to something (say 30%) if it is not specified?

In parallel, if there is no width for the col, should this just give a warning, or an error?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/pull/354#issuecomment-237426628, or mute the thread https://github.com/notifications/unsubscribe-auth/ABy2cit1qtOwg4ghwAeZGXqDQJ4GCiVsks5qcUJTgaJpZM4JZTdI.

Alex-Jordan commented 7 years ago

David, can you add some css for this? Things needed:

There is a table here where you can see the effect of fiddling with bottom margins: http://spot.pcc.edu/~ajordan/MBX/section-13.html#table-time-units

And there is a table here where the next-to-last column has the "justify" class. http://spot.pcc.edu/~ajordan/MBX/section-13.html#table-18 It's almost too narrow to see the effect of justifying text, but you can just barely see the effect in the top right cell.

Alex-Jordan commented 7 years ago

For a paragraph column in a tabular in a sidebyside panel, the @width should interpreted as a fraction of the panel's @width. So you'll need to adjust the template for a tabular in a sidebyside to pass a (restricted) with into the general tabular template, so the correct pixel count is used.

Do I actually have to do this twice? Once for sidebyside/tabular, and again for sidebyside/table/tabular?

davidfarmer commented 7 years ago

Alex,

I prefer "j" to "justify". I added CSS for both, and will delete "justify" once you make the change.

I did paragraph spacing the way I suggested for lists: remove the margin-bottom of all p in tables add a margin-top to p+p in tables

I made that margin-top be 1em (that is less than 0.9*1.25 but I thought it looked big enough)

A few issues:

The cell:

Colspan=2 lef mid with lines

has "br"s in it, but without the closing slash. I'n not sure why they are there (since the point of "lines" is to not wrap white space). But if you want them, then should be self-closing tags.

Other cells with "lines" also have the "br"s, so I am not sure if there is a cell that actually demonstrates what "lines" does.

In the paragraph after Table 13.14, the word "Section" is missing before the link "8.3".

David

On Thu, 4 Aug 2016, Alex Jordan wrote:

David, can you add some css for this? Things needed:

  • a class for test-align:justify; (I'm calling it "justify", but it is a peer of "l", "c", and "r", so perhaps "j"?)
  • mathbook-content p has margin-bottom:1.25em (and other margins 0). This messes with alignment inside a table cell, and I think we want the last p within a td to have margin-bottom:0. (But only the last.)
  • for p within a td that are not last, the bottom margin should perhaps be smaller than 1.25em. The font is scaled down to 90% within a table, so perhaps 90% of 1.25em?

There is a table here where you can see the effect of fiddling with bottom margins: http://spot.pcc.edu/~ajordan/MBX/section-13.html#table-time-units

And there is a table here where the next-to-last column has the "justify" class. http://spot.pcc.edu/~ajordan/MBX/section-13.html#table-18 It's almost too narrow to see the effect of justifying text, but you can just barely see the effect in the top right cell.

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

Alex-Jordan commented 7 years ago

I prefer "j" to "justify". I added CSS for both, and will delete "justify" once you make the change. I did paragraph spacing the way I suggested for lists: remove the margin-bottom of all p in tables add a margin-top to p+p in tables I made that margin-top be 1em (that is less than 0.9*1.25 but I thought it looked big enough)

OK, thanks. Looks good. And I have re-written the XSL so that there will not be class="l ... j" but just plain class="j ...".

without the closing slash

I see that but I don't understand why. Here is the XSL that creates those, which is the same structure as how br is added elsewhere:

<xsl:template match="mathbook//tabular//line">
    <xsl:apply-templates />
    <!-- is there a next line to separate? -->
    <xsl:if test="following-sibling::line">
        <br />
    </xsl:if>
</xsl:template>

Should the one line be?

<xsl:element name="br" />

I'm not sure why they are there (since the point of "lines" is to not wrap white space).

If you only want one line of non-breaking content, with this PR that is accomplished by doing nothing but putting the content into the cell. This will be a change in behavior, but it makes the HTML behavior match the LaTeX behavior. I mentioned this recently in the forum thread.

If you use <line>s within a cell, there will be <br /> at the end of each line. And the "lines" class does break at <br />. The purpose is if the author has a special need for breaking to happen at a designated place, like I did here: http://spot.pcc.edu/math/clm/to-the-instructor.html

If you take a cell with the "lines" class and make the screen narrow, you will see that there is no breaking between words. For example, at http://spot.pcc.edu/~ajordan/MBX/section-13.html#table-18, the "Lef mid" in the upper left will not break between those words if the screen gets narrow. Arguably you might want it to break, but the point is that the LaTeX would never break in this situation, and authors end up oblivious to the fact that their LaTeX tables are running wide.

rbeezer commented 7 years ago

Main existing tabular

  1. Give it a parameter, "width"?, "overall-width"? Set to 100% as default with "select". Use that to scale the 532px to 532px and run with that as basis for widths of paragraph colunms.
  2. This should look stupid and be status quo.

Tabular inside sidebyside "panel-box" routines.

  1. Pass width of panel (string with %) into call of main tabular.
  2. Possible you need to pass panel-width down into "panel-box"

Is that sketchy enough?

Rob

On 08/04/2016 09:29 PM, Alex Jordan wrote:

For a paragraph column in a tabular in a sidebyside panel, the @width
<https://github.com/width> should interpreted as a fraction of the panel's
@width <https://github.com/width>. So you'll need to adjust the template for
a tabular in a sidebyside to pass a (restricted) with into the general
tabular template, so the correct pixel count is used.

Do I actually have to do this twice? Once for |sidebyside/tabular|, and again for |sidebyside/table/tabular|?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/pull/354#issuecomment-237752506, or mute the thread https://github.com/notifications/unsubscribe-auth/ABy2ctLvLhC87k4pbvKphPq_oEmPEWNgks5qcrw2gaJpZM4JZTdI.

davidfarmer commented 7 years ago

I understand now about the examples you have for "lines". I guess I was expecting an example where a column is visibly wider, because it contains lines content that did not break.

If you only want one line of non-breaking content, with this PR that is accomplished by doing nothing but putting the content into the cell. This will be a change in behavior, but it makes the HTML behavior match the LaTeX behavior. I mentioned this recently in the forum thread.

If you use s within a cell, there will be
at the end of each line. And the "lines" class does break at
. The purpose is if the author has a special need for breaking to happen at a designated place, like I did here: http://spot.pcc.edu/math/clm/to-the-instructor.html

If you take a cell with the "lines" class and make the screen narrow, you will see that there is no breaking between words. For example, at http://spot.pcc.edu/~ajordan/MBX/section-13.html#table-18, the "Lef mid" in the upper left will not break between those words if the screen gets narrow. Arguably you might want it to break, but the point is that the LaTeX would never break in this situation, and authors end up oblivious to the fact that their LaTeX tables are running wide.

Alex-Jordan commented 7 years ago

Made another commit. Still no normalization of percentages. And just now realizing still no check that percentages sum to <= 100%. But I think the rest is addressed. Take a look, or wait til I come back to the sum check.

David, you can remove the "justify" class. It's using "j" now.

davidfarmer commented 7 years ago

Done

David, you can remove the "justify" class. It's using "j" now.

Alex-Jordan commented 7 years ago

Apparently not closing br is proper HMTL. Same for link, img, and meta judging by some tags in MBX HMTL output that are not being closed by XSLT.

http://stackoverflow.com/questions/3558119/are-self-closing-tags-valid-in-html5

davidfarmer commented 7 years ago

My reading of that answer is that closing the br tag is also valid, since it is a void element.

Since Firefox (on my computer) supplies the closing tag, my suggestion is that MBX output a self-closing tag for those void elements.

On Mon, 8 Aug 2016, Alex Jordan wrote:

Apparently not closing br is proper HMTL. Same for link, img, and meta judging by some tags in MBX HMTL output that are not being closed by XSLT.

http://stackoverflow.com/questions/3558119/are-self-closing-tags-valid-in-html5

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

Alex-Jordan commented 7 years ago

I do not think we can make the closing tag without circumventing clean xsl.

We are using method="html" for the HTML output, and xsltproc apparently knows when to exclude closing tags. It excludes them for link, meta, img, and apparently br as well. None of the following in the xsl will give a closing br tag:




/xsl:element xsltproc processes these html elements this way when method="html". (Caveat: I did not literally check all of these. I did however check the same variants with "link" instead of "br" when I was learning about this. And the first one here is what is currently in use, and it would seem that if xsltproc is coded to leave out the close in that case, why wouldn't it do so for all?) With method="xml", you get closing tags. But I saw directly this is really bad for the link elements. None of the css was working after I closed the link tags to the css files. The only thing I can think if is literally writing them out with <br />/xsl:text It seems like a red flag to have to do this though. And the answer at that post indicates that "
" might break HTML4 browser. On Mon, Aug 8, 2016 at 5:02 PM, davidfarmer notifications@github.com wrote: > My reading of that answer is that closing the br tag is also valid, > since it is a void element. > > Since Firefox (on my computer) supplies the closing tag, my suggestion is > that MBX output a self-closing tag for those void elements. > > On Mon, 8 Aug 2016, Alex Jordan wrote: > > > Apparently not closing br is proper HMTL. Same for link, img, and meta > > judging by some tags in MBX HMTL > > output that are not being closed by XSLT. > > > > http://stackoverflow.com/questions/3558119/are-self- > > closing-tags-valid-in-html5 > > > > — > > You are receiving this because you commented. > > Reply to this email directly, view it on GitHub, or mute the > > thread.[AAM6LCTwmU28EZKhtFxQs_z6DmYx3UuAks5qd7JUgaJpZM4JZTdI.gif] > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > https://github.com/rbeezer/mathbook/pull/354#issuecomment-238414458, or mute > the thread > https://github.com/notifications/unsubscribe-auth/AEg3AHRxzZeRS0wT9McvRo281Bl0Isuvks5qd8OdgaJpZM4JZTdI > . ## Alex Jordan Mathematics Instructor Portland Community College
Alex-Jordan commented 7 years ago

OK Rob, ready for you to take a look.

My XSL is getting better. cap-width-at-one-hundred-percent template almost worked on first try except for a typo.

rbeezer commented 7 years ago

Thanks, Alex. May peek soon, but at AIM right now, so may not get totally serious until the weekend.

Yes, your XSL has been improving rapidly for a quite some time. Get a copy of Lenz' book and read it carefully (very short) if you have not already. I think you are ready to get a huge boost out of it.

Rob

On 08/09/2016 06:24 PM, Alex Jordan wrote:

OK Rob, ready for you to take a look.

My XSL is getting better. |cap-width-at-one-hundred-percent| template almost worked on first try except for a typo.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/pull/354#issuecomment-238741131, or mute the thread https://github.com/notifications/unsubscribe-auth/ABy2crG53d34LFcHVjNsWamgPnDrODAoks5qeShfgaJpZM4JZTdI.

rbeezer commented 7 years ago

I think this is ready to go. One suggestion, one question. I'm happy to make changes as part of merging this - just let me know what you think.

In the maximum-width routine, I think

<xsl:if test="count($nodeset) &gt; 0">

can become just

<xsl:if test="$nodeset">

Two places, close by each other, you have:

match="mathbook//tabular"
match="mathbook//row"

What is the purpose of the mathbook// parts? I think that is almost entirely ineffective?

Alex-Jordan commented 7 years ago

If you think <xsl:if test="$nodeset"> will work, then great. Counting was more literally what my brain was thinking, but XSL clever use of emptiness is fun. Have you tested? Is it possible to have a $nodeset that is empty pass that test just because it has been given a name, or some such thing?

As for mathbook//, it wasn't for nothing, I can say that much. I cannot recall exactly what happened because it was so long ago, but at some point something did not work until I added at least one of those mathbook//s. I'm nearly certain it was about widths getting passed down the line from a sbs panel to a tabular. [Recall that this whole thing ultimately makes the HTML tds have inline CSS measured in pixels, not %, based on the 600px design width. For a paragraph column inside a tabular inside a sbs, the calculation requires multiplying 600px by two percentages. The tabular has to not only know % widths of its paragraph cols, but also the % width of the ambient sbs panel.]

At some phase of testing, widths just were not being passed down, even though by rights, it appeared to me that they should have. Some forum post somewhere suggested adding the root ancestor to the match. I did, and it worked. (I should have commented the details; live and learn.)

I guess we could remove the mathbook// and then do thorough testing. I just did light testing and found no issues. Either

  1. something will go wrong in some (possibly corner case) sbs scenario, or
  2. the issue became moot as other things were edited. Possibly during the same editing session and so there might not even be a commit where the issue I saw could be repeated.
Alex-Jordan commented 7 years ago

OK, after a good night's sleep, let me walk back some of my "nearly certain"ness.

Ultimately we required the author to give width for a paragraph column. But there was a time when the author could give no width, and I was having it deduce the width by counting cols in the tabular. Or, if there were no cols, by counting cells in a row.

There was a template in mathbook-common that did this. And I think this is the thing I couldn't get to work until I added mathbook//s. That template is gone.

In short I think you can take those mathbook//s out. But if you're ever writing XSL and something is not working when you think that it really should be working, apparently there are (rare) situations where the match needs its root element.

rbeezer commented 7 years ago

Sleep is good. ;-)

Lenz says: "a node-set converts to false if the node-set is empty, true otherwise." I'll simplify.

Well, that all sounds highly suspect to me. There's a lot of crazy XSL suggestions out there from folks who clearly don't know what they are doing. I've learned the hard way. ;-)

I'm not even sure you are at the root, that'd mean writing match="/mathbook//tabular" with the leading slash taking you to the top of the source tree. Anyway, I'll test without, and parse my way through the width parameters transiting down into the templates. Thanks for the reply.

rbeezer commented 7 years ago

Behaving quite well with discussed changes. Working great, in fact. One more cruise before I clean-up and merge.

Would you like the honors to announce, once I'm done? That's an invitation, not a request. ;-)

Alex-Jordan commented 7 years ago

Please go ahead and announce. Only having spotty time to be online today. Bob Plantz shoudl get a poke since he needs this in his book.

Thanks for reviewing and testing! Pretty sure the quirk I ran into was with the abandoned scheme for auto-counting columns and cells. Then it would make sense that I did it for tabular (col children) and row (cell children) but not other table-related elements.

On Sun, Sep 25, 2016 at 12:40 PM, Rob Beezer notifications@github.com wrote:

Behaving quite well with discussed changes. Working great, in fact. One more cruise before I clean-up and merge.

Would you like the honors to announce, once I'm done? That's an invitation, not a request. ;-)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/pull/354#issuecomment-249441922, or mute the thread https://github.com/notifications/unsubscribe-auth/AEg3AEoVGA5ZLuDOlhmIZuFDsFrdU1ANks5qts4xgaJpZM4JZTdI .

Alex Jordan Mathematics Instructor Portland Community College

rbeezer commented 7 years ago

Merged with two changes discussed above simply added into the one commit. Thanks very much for this! (And your patience!)