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

Attempt to fix verticle alignment of some boxes in latex backend #265

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wilbowma
Copy link
Contributor

This addresses #260 by providing a default box-adjuster. This requires an extra LaTeX package.

I'm not really sure this is a good solution, but I thought I'd offer a patch for experts to review. It might have unintended consequences if existing code was relying on ignoring the vertical alignment style property. The tests seem to pass, but it seems like a hard thing to write a test for.

@mflatt
Copy link
Member

mflatt commented Aug 19, 2020

Adding a new package is always worrying, but I'd be happy to have a solution here.

Would using minimal instead of export work and potentially avoid unwanted interactions with uses of includegraphics, or is export reasonably unlikely to break existing documents?

@rfindler Do you think we should give this a try and check the document rendering and a few papers?

@rfindler
Copy link
Member

I tried the documentation PDF build and it caused no problems there. I also built a recent paper and it didn't cause any problems there either. I'm not savvy enough to understand how this fixes the problem but it sounds like you both are so I'm in favor!

@wilbowma
Copy link
Contributor Author

minimal is probably better; I was explicitly trying to avoid using it directly with includegraphics directly.

@rfindler, what was happening was the call to do-render-paragraph did specify the box should be top aligned by passing 't as as-box-mode. However, for paragraphs, this gets implemented by reading the style properties and looking for a box-mode implementation. The pict didn't have one, so it was wrapped in hbox, the default, which coverts to a box but doesn't adjust vertical alignment. This patch uses adjustbox as the default, which both converts to a box and adjusts vertical alignment.

@wilbowma
Copy link
Contributor Author

When I try minimal, I get complaints that valign is undefined by adjustbox, which is strange.

I also notice a small difference, although I haven't observed it yet: \adjustbox{valign=c} should probably behave like \hbox, since that's what it's replacing. But I think it behaves like \mbox. Unfortunately, I don't really understand how these two things differ.

@wilbowma
Copy link
Contributor Author

The vertical alignment in tables does not behave as expected. It seems to be only coincidence that my example worked. I'll continue debugging and post in #260, but this patch doesn't really work.

@mflatt
Copy link
Member

mflatt commented Aug 21, 2020

Thanks for investigating more. My vague memory of table-cell alignment was that the CSS and Latex models were deeply different, and I found some approximation that worked for some common cases and gave up on the rest.

@wilbowma
Copy link
Contributor Author

wilbowma commented Aug 21, 2020 via email

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.

3 participants