search

rrthomas / hpmor

PDF, ePUB and Mobi versions of “Harry Potter and the Methods of Rationality”, from LaTeX source
http://hpmor.com/
295 stars 55 forks source link

Fix McGonagall's whiteboard in ebooks #157

Closed wfdewith closed 7 months ago

wfdewith commented 7 months ago

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 commented 7 months ago

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

entorb commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

entorb commented 7 months ago

Thanks!