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

Fix McGonagall's whiteboard in ebooks #157

Merged
merged 3 commits into from
Jan 24, 2024

Conversation

wfdewith
Copy link

Currently, it renders as <p>-1ex</p>, which was somewhat confusing the first time I encountered it.

I think the \begin{center} environment does not actually work in the current epub, but that is a different problem.

@rrthomas
Copy link
Owner

Thanks for this! LaTeX looks good to me; @entorb, what about the ebook side?

@entorb
Copy link
Collaborator

entorb commented Jan 24, 2024

That you very much for the fix! Tested it successfully in PDF and ebook.

Just added a slight modification: using a hack to map that command to the already existing css class via a fake color name.

@wfdewith
Copy link
Author

Just added a slight modification: using a hack to map that command to the already existing css class via a fake color name.

Ah, so that's why those textcolor commands exist. I see that the CSS already contains a definition for it, but that doesn't center it, which I think would improve it even more.

@entorb
Copy link
Collaborator

entorb commented Jan 24, 2024

but that doesn't center it, which I think would improve it even more.

Good point. I just checked the current output. It is a <span> so just adding a text-align: center;to the css would not work. One would need to adjust the LaTeX \renewcommand to yield a <div>. I tried surrounding it with a \begin{center}, that did not work. Feel free to propose a change it you like. Otherwise it would be fine for me to merge as it is now.

PS: you can use docker to perform the ebook creation if you do not want to install the required packages globally.
docker run -it --mount type=bind,src="$(pwd)",dst=/app hpmor ./scripts/make_ebooks.sh
(it requires a hpmor.pdf already present in the dir, to extract the cover image from)

@wfdewith
Copy link
Author

Good point. I just checked the current output. It is a so just adding a text-align: center;to the css would not work. One would need to adjust the LaTeX \renewcommand to yield a

. I tried surrounding it with a \begin{center}, that did not work. Feel free to propose a change it you like. Otherwise it would be fine for me to merge as it is now.

I'll experiment a bit to see if I can make it a div.

@wfdewith wfdewith force-pushed the mcgonagall-whiteboard branch from f4b4c33 to 757a773 Compare January 24, 2024 18:45
@wfdewith
Copy link
Author

@entorb I got it, it nicely renders as a div now, with centered uppercase text.

@entorb entorb merged commit d564954 into rrthomas:main Jan 24, 2024
1 check passed
@entorb
Copy link
Collaborator

entorb commented Jan 24, 2024

Thanks!

@wfdewith wfdewith deleted the mcgonagall-whiteboard branch January 25, 2024 07:36
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