Conversation
The logo is placed at the top of the navication sidebar.
|
So it's still waiting for you to fix the review comments, right? |
… sidebar, at fix width of 40% the sidebar width, and actually shows no matter the depth level of the current page
|
I think I am good feature-wise, I'll write some tests next. I am not familiar with goml at all, but I will try to take care of that soon |
|
So if I understand correctly, goml is used here to simulate user input? In that case I assume I should only need to write tests under |
|
It's used for GUI tests more globally. It allows you to query HTML states. It's easier to read than comparing plain HTML. |
|
@GuillaumeGomez I believe it is ready for review Another thing that I am not sure about is, if the logo does not exist, the book will still build without a warning |
|
If it's a local file, we should definitely fail the build. |
…se supporting this claim
|
@GuillaumeGomez I have addressed your comments, and added a
|
|
One question I just stumbled upon: is the logo image copied to the generated book? Because if not, it should (and it should be tested of course :3). |
|
How does this currently handle wide images? Could it function as a banner above the book title? |
|
Oh that's actually a good point: how do we handle images too big? |
Currently logo width is set at a fixed 20%, and the title's max width to 70%, wrapping if too long ; check above to see what it looks like Could be an option, let me know what you think!
Nice catch, I am adding it to the static files |
|
Image not visible. Please add a test with a very large image too (doesn't need to be a big image, just one with like 1px of width/height and like 1000px of height/width). |
And what happens if the logo doesn't live in the |
|
Right now, just like regular pictures, if it lives outside of src, then it won't be copied to the output directory, which kind of makes sense to me - my way to go is to make projects "self-contained", without links to the outside However if you prefer to force copy it, I have a implementation ready for review leveraging And if this is overkill, maybe we can just copy it to the root of the book with a name unlikely to make a collision, such as mdbook_logo. |
|
Huuuuum... Can we error instead if the picture is not part of the |
Add tests and test files: - large and tall images don't mess up layout - logo config is validated, or error is thrown, regardless of the src directory Update logo config documentation
|
@rustbot ready Full recap:
and I think that is pretty much it! |



Supersedes #1584, fixes #877
I have not addressed the comments (I have just rebased the original commits on top of
mainand fixed the failing tests, preserving the original author's contribution):An SVG would be prefered; maybe
mdbook-html/front-end/images/favicon.svgcould be a good candidateAlso there is no test at all at the moment.
Done during rebase
This has not been addressed further ; a simple default value could be
alt="{{ book_title }}"