Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make the theme responsive #46
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
Make the theme responsive #46
Changes from 20 commits
7aba474
203b40a
10f4349
ff61d60
7640739
ecf73cf
43da2b4
51a0c7a
efd1ec2
7953f1b
6daa876
250dbf6
5ffa114
e1ee8bd
1e182f3
611a1dc
783b19a
d31371e
190931b
b6fc448
cd92c8c
ed7b948
d96713e
bd1d802
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
pathto(master_doc) would be more appropriate here?
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.
Or does CPython documentation use contents for the toctree?
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.
Thank you for such a quick review! :) I know what I added doesn't work for sure. Let me try pathto(master_doc).
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 need help!
I don't know how to add the correct link to the
docs.python.org/3.9/
.In line 88 in layout.html I added
<a href="{{ pathto(theme_root_url) }}"
, and it createsfile:///.../doc-html(3)/3.9/https://www.python.org/.html
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.
Don't wrap it in pathto, just
{{ theme_root_url }}
is enough.That should fix it. :)
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.
In other words, that is behaviour I'd expect and I thought that's what you wanted?
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 guess I was confusing the two kind of links (to the docs home and the language/framework home). On the sites I used a lot lately (eg Vue.js) the style of the framework home page and the style of the documentation homepage are the same. And it feels like I just go to the documentation home, not the general home, when I click on the logo.
But you are right, now we have the correct behavior, linking to python.org.
We still don't have a way of going to the documentation homepage,
docs.python.org/3
from other pages on mobile. Is it necessary to add it, and add it in this PR specifically?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.
If you want to have a link to the documentation homepage, that'd be
{{ pathto(master_doc) }}
or{{ pathto("index") }}
. No need for a |e on either of them.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.
Honestly, this feels like something we can do in a follow up PR.
I'd really like to push for getting this landed soon, so that we can start moving other things forward without needing to come back and update this.
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.
Please, let me know if I need to change anything? I would be happy to get this landed soon, too!