-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Add mdbook docs page #11849
Add mdbook docs page #11849
Conversation
This shows how to host docs with mdbook. Requires #11848 to not need a reshim
29299af
to
bf5162f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
There are some things we should add to this guide:
- Integration with search to trigger ours when clicking on the search icon. Similar to what we wrote for MkDocs: https://docs.readthedocs.io/en/latest/intro/mkdocs.html#configure-read-the-docs-search
- Add custom CSS for flyout size
- Our search seems broken in the example. You cannot type the
s
letter for example. - It would be good to add a small paragraph at least in the
test-builds
project 😄 . It only says "Chapter 1"
https://github.com/readthedocs/test-builds/tree/mdbook | ||
|
||
Demo | ||
https://test-builds.readthedocs.io/en/mdbook/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should add custom CSS files to make the flyout looks nice; it's pretty small now:
See an example for MkDocs: https://docs.readthedocs.io/en/latest/intro/mkdocs.html#adjust-the-flyout-menu-font-size
os: ubuntu-lts-latest | ||
tools: | ||
rust: latest | ||
commands: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried using build.jobs.build.html
instead? I think we should be recommending as much as possible that approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't, but thought this was blocked on #11819
@humitos Should we be doing similar default configuration for the addons CSS selectors here, where we're able to detect the doctool and configure the Addons font size for example? It seems bad to punt this to the user if we know there's specific config we want for a tool, and we have the functionality to be able to detect the tool? 🤔 I feel like we're gonna get closer and closer to having magic on the front end if we keep adding in all these special cases tho, but I think it's unavoidable if we want to provide 💯 integrations for some of these tools. |
Co-authored-by: Manuel Kaufmann <humitos@gmail.com>
Yes. I've been on the fence about adding those CSS rules automatically; but since they are |
Use our heuristic to detect the documentation tool/theme and add specific `--readthedocs-*` CSS variables based on that for known tools/themes. Reference: readthedocs/readthedocs.org#11849 (comment)
.. code-block:: css | ||
:caption: readthedocs.css: | ||
|
||
:root { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@humitos This worked nicely in my test: https://test-builds.readthedocs.io/en/mdbook/
But it seems like the s
key is hosed when search is open, because they're hooking into that to trigger their own search. 🙃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
I remember having a similar issue with Material for MkDocs. They ended up fixing the issue on their side by skipping handling the keydown if the focus was inside a text input.
The options I see here are:
- Disable their own search
- Disable their hotkeys
I'm not sure if that's possible, tho.
I'd like to get this merged so we can start ranking for it, and then futz with the final details of integration? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, we can merge this.
There are some points we want to do after:
- Continue the investigation for auto-CSS (Add styles based on the documentation tool addons#473)
- Disable their search / Fix the
s
key - Migrate the YAML example to
build.jobs.build.html
Looks like the code for this is here: https://github.com/rust-lang/mdBook/blob/master/src/theme/searcher/searcher.js#L331-L335 Not really sure how to handle this -- I know we hit something similar with Mkdocs Material, but don't remember how we solved it. |
They solved on their side. I commented this in the previous thread 😄 : #11849 (comment) |
I opened rust-lang/mdBook#2507 |
This shows how to host docs with mdbook.
Requires #11848 to not need a reshim (example: https://github.com/readthedocs/test-builds/blob/mdbook/.readthedocs.yaml#L10)
📚 Documentation previews 📚
docs
): https://docs--11849.org.readthedocs.build/en/11849/dev
): https://dev--11849.org.readthedocs.build/en/11849/