PreTeXtBook / pretext

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

Make WeBWorK image widths be percentages #736

Closed Alex-Jordan closed 6 years ago

Alex-Jordan commented 6 years ago

This will need a deprecation warning/alert. And it's not backward compatible: WW image widths need to be percentages now like everywhere else. Also, this should allow you to tidy up the schema some.

After this, I can PR the new extraction style sheets and the new webwork component for the mbx script.

Alex-Jordan commented 6 years ago

You'll need to kill your local copy of this branch, because of the commit message edit in the previous PR.

rbeezer commented 6 years ago

Off-duty tomorrow, so message will need to wait for Thursday.

Please take a look at the mode="get-width-percentage" template in -common. it'd be best to use that so we get consistent behavior. Generally the code passes around strings (with % iirc) and the receiver figures out what to do with them. Defaults are 100% which is usually grossly too big, so authors recognize they should do something about it.

Alex-Jordan commented 6 years ago

I force pushed with the suggested edits. But I see that I will have to do so again. So wait until I say something, or Thursday, whichever comes later :)

rbeezer commented 6 years ago

No problem.

When I fetch (not pull) I rename the branch to something that makes sense to me locally. I don't think new commits up on GitHub cause me any problems, so long as I haven't built on top of the old ones locally in the meantime. Even then, if they are not too intermingled, I can probably work it out.

I can probably work up the deprecations independently anyway. But not tonight.

On 09/26/2017 09:13 PM, Alex Jordan wrote:

I force pushed with the suggested edits. But I see that I will have to do so again. So wait until I say something, or Thursday, whichever comes later :)

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

Alex-Jordan commented 6 years ago

OK, I do have questions. In WW, an image is never in a figure. It's only a (single) child of a sidebyside. The common mode="get-width-percentage" does not apply, unless I extend it to match on something like webwork//image.

I considered adding the full sidebyside machinery to mathbook-webwork-pg.xsl. But (a) that sounds like a lot of work and testing for something that is going to be trashed from the repo next week.

And (b), I'm not sure how it would work. In HTML and LaTeX, the sidebyside's "widths" attribute is used in building the container of the image(s). It's not used when the image itself is written to the HMTL or LaTeX. Or am I wrong about that? Because in PG, it can't be done that way. The image itself needs to know the specified width. There is no notion of a surrounding container whose widths can be controlled. The command that invokes the image needs to ship out a specified width right then and there.

So I'm having trouble seeing how a full sbs implementation would work for PG. Because if it can't, I keep the simple sbs template I have now for PG, which merely applies image and tabular templates. Should I forget about sbs machinery, but still ask authors to specify their widths in the parent sbs? And the image uses its parent's "widths" to declare its width?

Alex-Jordan commented 6 years ago

In other words, which is right for within a webwork?

<sidebyside>
    <image pg-name="$gr[0]" width="25%"/>
<sidebyside>

or

<sidebyside widths="25%">
    <image pg-name="$gr[0]"/>
<sidebyside>

where if using the latter, the XSLT is not going to be the full mechanism you've developed elsewhere. The image would just look to its parent's @widths, maybe check that there is only a single percentage there, and use that.

rbeezer commented 6 years ago

Too late to be too coherent, but this sounds right. Let's be consistent, so authors don't need to remember different ways of doing things. We'll just want to warn about multiple items in a "sidebyside" (later).

Look around - the template that gets the width looks outward to the enclosing sidebyside, perhaps via computing a $layout, or something like that. See if you can avoid doing too much duplicative until you need to convert a percentage into a pixel count.

On 09/26/2017 09:48 PM, Alex Jordan wrote:

or

| |

where if using the latter, the XSLT is not going to be the full mechanism you've developed elsewhere. The |image| would just look to its parent's |@widths|, maybe check that there is only a single percentage there, and use that.

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

Alex-Jordan commented 6 years ago

OK, ready for you to look at again.

Common did not have a "get-width-percentage" template matching on image[ancestor::sidebyside]. But LaTeX did. Both "regular" images and, now, webwork images use this template in the LaTeX production.

Common had "get-width-percentage" matching on video[ancestor::sidebyside]|jsxgraph[ancestor::sidebyside]. And it was identical to the one for image[ancestor::sidebyside] in latex. And nothing else in common matches on image[ancestor::sidebyside] with that mode.

So I hope I didn't overlook something, but I removed the one from the latex style sheet and just added the additional match to common. I'm going to need that template for the forthcoming extract-pg.xsl and extract-pg-ptx.xsl style sheets too, so common seems appropriate.

Alex-Jordan commented 6 years ago

To list them here in one place, I think the deprecations are that webwork//sidebyside/image[@width] moves to webwork//sidebyside[@widths]/image.

And instead of being a pixel count, it's a percentage.

And maybe now is when you'd add some sort of alert that in this context, widths should only have on width and the sbs should only have one tabular or image child.

And this all has ramifications for the schema that you understand far better than I.

rbeezer commented 6 years ago

Down the rabbit hole, and back. mathbook-webwork-pg.xsl gets included by mathbook-html.xsl and the definition of

<xsl:template match="image[ancestor::sidebyside]|video[ancestor::sidebyside]|jsxgraph[ancestor::sidebyside]" mode="get-width-percentage">

copied into mathbook-webwork-pg.xsl is clobbering the mathbook-html.xsl version. Eventually moot, I guess. I'll need to sleep on how to work around it.

rbeezer commented 6 years ago

Perhaps the final template in the chain of overrides/replacements needs a and ancestor::webwork in it? I'll test later.

Alex-Jordan commented 6 years ago

Most templates in webwork-pg.xsl start the match with "webwork//..." for precisely this kind of reason. You could add that before each option, or use "and ancestor::webwork". Either way. Sorry for the oversight.

On Wed, Nov 8, 2017 at 9:48 PM, Rob Beezer notifications@github.com wrote:

Perhaps the final template in the chain of overrides/replacements needs a and ancestor::webwork in it? I'll test later.

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

-- Alex Jordan Mathematics Instructor Portland Community College

rbeezer commented 6 years ago

OK, immediate problem solved. Limiting to webwork restored HTML behavior. More testing/review, then will work on deprecations.

Sorry for the oversight.

No problem. I was being spectacularly dim-witted about debugging it.

rbeezer commented 6 years ago

Looking good. Miscellaneous comments:

I'm going to work on deprecation. It will be a warning only, since it looks too complicated to try to fix up old style to new style, especially due to side-by-side (plus I'll do schema).

Holler soon if above needs small changes, but I'll likely proceed no matter what.

Rob

Alex-Jordan commented 6 years ago

There is a hard-coded 600 in mathbook-webwork-pg.xsl. Is that the HTML design width or something else? Is it permanent or temporary? Could be better as a global variable defined very early in the file?

It is the HTML design width. My suggestion:

  1. Leave the hard 600 in the current mathbook-webwork-pg.xsl which will be gone by the time this is all done with.
  2. Move the design-width from mathbook-html.xsl to mathbook-common.xsl. Because our future has this number independently being used by mathbook-html.xsl, extract-pg.xsl, and extract-pg-ptx.xsl. Unlike the current mathbook-webwork-pg.xsl, each of these three do import common.

If you give the thumbs up, you could just do nothing now and I'd do item 2 when I introduce the PR with extract-pg.xsl,and extract-pg-ptx.xsl.

Testing shows a [ ....] where the first space goes away, and it appears new version has new space at the very end. Seems OK, but thought to mention it.

Are you easily able to tell me what's in the ...? I could assess if it matters.

Not related, but code says webwork.server.latex is assumed to end with a slash. Do we check that at input? Could we? Better to save authors the confusion.

I'm inclined not to invest much here since webwork.server.latex is going away. If I introduce something else that assumes a slash in one of the upcoming PRs, can we look at that instead when it comes up?

rbeezer commented 6 years ago

Sounds good. No point in fixing up small details if they are going away.

Lost the stray spaces example, which is why it had ... ;-) We'll not worry about it now, but maybe it'll turn up somewhere along the way. I'm going to wrap up this one. Thanks for the reply.

Alex-Jordan commented 6 years ago

Question: when I move $design-width to common, should it keep that name? It has the value 600, but nothing about the name or the value indicates this is a pixel measurement. The commentary does. Just thinking that now this commentary will be in the common file, and actual use of the variable will be in 3+ other files.

On Thu, Nov 9, 2017 at 11:54 AM, Rob Beezer notifications@github.com wrote:

Closed #736 https://github.com/rbeezer/mathbook/pull/736.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/pull/736#event-1334741344, or mute the thread https://github.com/notifications/unsubscribe-auth/AEg3AE-YlgSC8NBPagZ_lmk1nTgL2Uzbks5s01iDgaJpZM4PlJnI .

-- Alex Jordan Mathematics Instructor Portland Community College

rbeezer commented 6 years ago

I thought about units on that at one point earlier today. I want to say that if the units are pixels then it should go in mathbook-html.xsl. Simple as that. This type of logic logic has spared me a lot of trouble and/or helped make the code more economical or robust.

If a stylesheet will not pick it up this way, then it should be defined there, even if it is 600 since I'd probably argue it is playing a slightly different role, and might someday change independently.