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

Make WeBWorK image widths be percentages #736

Closed
wants to merge 2 commits into from

Conversation

Alex-Jordan
Copy link
Contributor

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

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

@rbeezer
Copy link
Collaborator

rbeezer commented Sep 27, 2017

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

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

rbeezer commented Sep 27, 2017 via email

@Alex-Jordan
Copy link
Contributor Author

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

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

rbeezer commented Sep 27, 2017 via email

@Alex-Jordan
Copy link
Contributor Author

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

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

rbeezer commented Nov 9, 2017

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

rbeezer commented Nov 9, 2017

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

@Alex-Jordan
Copy link
Contributor Author

Alex-Jordan commented Nov 9, 2017 via email

@rbeezer
Copy link
Collaborator

rbeezer commented Nov 9, 2017

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

rbeezer commented Nov 9, 2017

Looking good. Miscellaneous comments:

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

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

rbeezer commented Nov 9, 2017

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

Alex-Jordan commented Nov 9, 2017 via email

@Alex-Jordan Alex-Jordan deleted the webwork-ptx branch November 9, 2017 21:52
@rbeezer
Copy link
Collaborator

rbeezer commented Nov 10, 2017

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.

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

Successfully merging this pull request may close these issues.

None yet

2 participants