PreTeXtBook / pretext

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

Image leavevmode #1957

Closed Alex-Jordan closed 1 year ago

Alex-Jordan commented 1 year ago

Two pictures are worth 2000 words. So, before in LaTeX:

Screen Shot 2023-03-18 at 2 30 32 PM

And after:

Screen Shot 2023-03-18 at 2 24 40 PM

This PR applies \leavevmode before atomic images that you would want to be in line with a number off to the left (from a list, exercise project). Then it backs up by an appropriate amount of space so the image is "top aligned" with the number. The "backing up" is either by 1.5\baselineskip or 0.5\baselineskip depending on the situation. These values are explained in the comments.

rbeezer commented 1 year ago

Two pictures are worth 2000 words.

Hah!

I can't test right now, so I'll ask. What is the behavior for HTML?

Alex-Jordan commented 1 year ago

Current HTML behavior could be better. But I think some minor CSS changes could handle these.

Screen Shot 2023-03-18 at 5 37 35 PM Screen Shot 2023-03-18 at 5 37 43 PM Screen Shot 2023-03-18 at 5 38 00 PM

I didn't take a screenshot, but the new task and exercisegroup behavior is like the list behavior from the earlier screenshots. Sepaarated thiis ini two commits so you can see the new examples before and after the real commit here.

rbeezer commented 1 year ago

Thanks (I'm at 35000 feet right now.) Will study more carefully once at a desk. A bit hard to judge with only the limited context in the diff. So take as first reactions:

Alex-Jordan commented 1 year ago

I can do first bullet and look into second.

Not sure I understand 3rd. Maybe it is not clear but status quo is that both HTML and LaTeX have some undesirable behavior, but they are different. LaTeX puts these images too high. HTML/CSS puts them too low. So it's not a situation where "at least they are in sync". Seems like we can make one better and independently make the other better and they will be in sync then.

Alex-Jordan commented 1 year ago

Frustratingly, writing \leavevmode\nopagebreak\vskip{-1.5\baselineskip} before \begin{image} does not produce exactly the same results as putting begin=\leavevmode\nopagebreak\vskip{-1.5\baselineskip} as an argument to the tcolorbox for an image environment.

So I have your idea for an additional argument to the image environment "working" but the spacing is not quite as good. And it is not consistent across all the types (list items, exercises, tasks, exercisegroup exercises), and doesn't seem to match my understanding of where -1.5\baselineskip and -0.5\baselineskip should be the exact values to use. Here is one pic to compare with the second pic from my original post:

Screen Shot 2023-03-18 at 9 45 01 PM

Maybe hard to see but the pics are now a bit too high. And I don't have screenshots to show but on the exercisegroup exercises they now are a bit too low. I've even been playing with combinations of placing %
 here and there too, and this is the best one.

If it has to be this way, it seems there is some internal thing about how tcolorbox begin=... works that I don't understand.

rbeezer commented 1 year ago

Frustratingly

Welcome to my world.

Have you seen the tcolorbox key: /tcb/box align? I see it described on p. 91 of the current manual. Maybe the base alignment would work here. Perhaps the \leavevmode might need to be independewnt of that? Or in before or ...

Bullet 3: I just meant to get this pretty much finalized before we get the HTML stuff wound up.

Alex-Jordan commented 1 year ago

I resorted to using a \NewDocumentEnvironment around the existing \NewTColorBox for images. In the former, I can include the leavevmode etc. without the TCB interference I described in my last post.

Alex-Jordan commented 1 year ago

Didn't see your last note. I'll look into box align.

rbeezer commented 1 year ago

And to further add to the mix, from p. 16 tcolorbox documentation:

Basically, \NewTColorBox operates like \NewDocumentEnvironment.

Just use a case-sensitive search. ;-)

Alex-Jordan commented 1 year ago
Basically, \NewTColorBox operates like \NewDocumentEnvironment.

I saw that and the current version of this PR is based on that. If you have nothing better to do (heh) take a look aat the current version.

\NewTColorBox does not offer the before-code and after-code argments that \NewDocumentEnvironment offers. Instead, it assumes all you want to do is begin and end a tcolorbox. And it gives you a place to pass options to that tcolorbox.

Alex-Jordan commented 1 year ago

I tried using box align=top, which puts the top of the image on the baseline of the line. So far so good.

The next step would be to "back it up" by one baseline skip. Everything I have tried with tcb options (before, before skip, and other things either does not work, or does not work as hoped for. For example, before skip=\notblank{#4}{-1\baselineskip}{} somehow affects the positioning of the list markers too, as you can see here:

Screen Shot 2023-03-19 at 12 37 11 PM

I have to give up on this. Let me know if you think the NewDocumentEnvironment approach is not too clunky or if you otherwise foresee issues with it.

rbeezer commented 1 year ago

\NewTColorBox does not offer the before-code and after-code argments that \NewDocumentEnvironment offers.

I sorta expected that would be the case.

I have to give up on this.

You aren't trying hard enough. ;-) ;-) ;-)

An extra depth of nesting structures in the preamble won't bother me. And the body is still just as semantic (nearly!) as before.

Latest commit looks good visually. Might do a round of merging (PRs, little stuff while traveling) later this afternnon. Since I have "have nothing better to do". ;-)

Thanks for working through this one.

rbeezer commented 1 year ago

Mysteries I'll never understand. Compare current PDF of sample article at website (or build yourself from current master), and the PDF generated with this PR.

Not a show-stopper and I think I'll merge this very soon.

Alex-Jordan commented 1 year ago

I have many versions of the PDF built:

In other words among all those, I get no changes in pagination.

Among the 5 commits are your LaTeX copyright commits. Are they in play for explaining that inconsistency? If you put this PR on top of current master, do you get the same inconsistency?

rbeezer commented 1 year ago

Well, I'd not added this PR onto current master. But no difference when I do.

copyright is only for book. ;-)

Not a problem, and I bet we have subtle differences in packages somewhere. Just a curiosity.

Thanks for all the checks.

Alex-Jordan commented 1 year ago

I just built the PDF from e938a31 (current master without this PR) and I still get that figure on page 150. But yeah, the public one has it on 149. So I guess it is some weird package difference.

Should we be concerned as developers that we don't get the same output? Could make future confusion if you and I are off by half a page in the sample article PDF.

oscarlevin commented 1 year ago

Regarding this oddity, this is an argument for requiring developers to use a fixed developer environment with common versions of latex and python etc. Codespaces can do that, which can also be set up locally as a docker image </bearpoking>

rbeezer commented 1 year ago

Merged, and now rebuilding website.

I really like how this ended up - the code and the feature.

Usual editorial work on the history. Sample images were great. Now that they have served their purpose, I moved them to follow the feature implementation.

Thanks!

rbeezer commented 1 year ago

<bearpoking> No kidding! ;-)

LaTeX is a strange beast. And sometimes counter-intuitive. "Not a problem" - and I'll go furhter, "I won't lose any sleep over this one."

davidfarmer commented 1 year ago

Did I miss the need for some corresponding CSS change, or is the HTML okay?