DOC [PST] conf, setup, general styling #28132
Conversation
Why pinning
|
|
|
There was a problem hiding this comment.
The problem has been fixed in pydata/pydata-sphinx-theme#1630 so we might be able to remove the pin soon.
Probably a good idea to keep pinning a specific version anyway.
Well, this is after all my personal taste. If maintainers do not like the scss or do not think the benefits of scss is worth the overhead of setting these stuff up, please feel free to let me know.
I'd like to see what @thomasjpfan and @GaelVaroquaux think on this. I personally don't know css at all anyway. The main question for me would be maintainability, in the sense that how many people know enough scss so that they can maintain this.
sphinx-remove-toctrees is for hiding the metadata routing section in the user guide from the toctree
Not sure why we need to do this.
| # Config for sphinx-remove-toctrees | ||
|
|
||
| remove_from_toctrees = ["metadata_routing.rst"] |
There was a problem hiding this comment.
This is because the current user guide hides this section from toctree, see screenshot below:
Screenshot of the current user guide page
See also @glemaitre's #26809 (comment):
Metadata routing has the wrong numbering in the "Section navigation" of the user guide. The reason is probably because it is supposed to be hidden in the old page.
|
Thanks for your review @adrinjalali!
Emm yes this is also true, especially if we tweak css a lot. But on the other hand I do see some nice improvements in 1.5.x. So my personal opinion is, we can postpone the decision (whether to pin a specific version or not) when finalizing, depending on how many customizations we made at the end.
Yep this is reasonable. I'm no expert either, thus only using the nested syntax but nothing else. If needed, here is an illustration:
|
|
I pushed a few additional commits into this PR:
@adrinjalali Since these changes are made after your approval, I want to confirm your opinion with these new changes. Footnotes |
|
#28132 (comment) makes sense to me. |
There was a problem hiding this comment.
Generally, I prefer scss over css, but I did not use it in scikit-learn because "its another thing to learn" for maintainers.
Currently, the landing page is not styled correctly. Is this expected?
|
Thanks for the feedback @thomasjpfan!
Makes sense, but I still want to argue that, if only using the nested selectors, I think most people can understand within a few minutes. Admittedly things like mixins and control directives are not that intuitive, but I actually am not using those either. And after all I think not many contributors would even touch this part of code. Of course, from another perspective, since I only uses nested rules when playing around it is easy for me to convert what I've done back into css, so either way works for me.
Yes, I plan to fix it in a follow-up PR, hoping to keep this one as small as possible. |
|
Update. Again there's something wrong: the primary sidebar would show up even when there is nothing to display. I will keep this as is because 0.15.2 contains some desired fixes, and hope to resolve the issue upstream so that we can bump again minimum to a higher version. |
|
I'm running a bit behind the regression of 1.4.0. But I promised that I'll take a look pretty soon. |
|
Thanks and no hurry @glemaitre. I'm currently keeping stashes of further changes so actually I'm not completely blocked by this one. |
|
So, after careful consideration I still included the general styling in this PR instead of leaving it to a further one, because when doing further work I found multiple patches relying on these. See the diff
Global+code.literal {
+ border: 0;
+}
Here I'm removing the border of inline code. This is in fact completely personal taste. The border may help distinguish inline code if background color is the same as that for inline code. However, I think the monospace font already suffices for such corner case. The main reason for me removing the border is that when there is a lot of such inline codes (especially on API pages) the borders are somehow very distracting for me. You may look at this page from Primary sidebar+.bd-sidebar-primary {
+ width: 22.5%;
+ min-width: 16rem;
+
+ // The version switcher button in the sidebar is ill-styled
+ button.version-switcher__button {
+ margin-bottom: unset;
+ margin-left: 0.3rem;
+ font-size: 1rem;
+ }
+
+ // The section navigation part is to close to the right boundary (originally an even
+ // larger negative right margin was used)
+ nav.bd-links {
+ margin-right: -0.5rem;
+ }
+}
Previously the primary sidebar seems a bit too large (taking 25% of page width) and I reduced a bit. Also the arrows (if exist) in the primary sidebar are too close to the boundary from my perspective, so I decrease the negative right margin (originally -1rem). The version switcher problem happens for mobile screen size, where there is a strange bottom margin. I also moved it away a bit from other icons and decreased the font size because of its large border. Please do not mind the "Other versions and download" section overflowing out of the sidebar. My current plan is to remove this section and integrate it into the version switcher. Article content+.bd-article {
+ h1 {
+ font-weight: 500;
+ margin-bottom: 2rem;
+ }
+
+ h2 {
+ font-weight: 500;
+ margin-bottom: 1.5rem;
+ }
+
+ // Avoid changing the aspect ratio of images; add some padding so that at least
+ // there is some space between image and background in dark mode
+ img {
+ height: unset !important;
+ padding: 1%;
+ }
+
+ // Override the pydata-sphinx-theme styling of .. topic:: directives
+ div.topic,
+ aside.topic {
+ background-color: transparent;
+ border: none;
+ padding: 0;
+ box-shadow: none !important;
+
+ p {
+ color: var(--pst-color-text-base) !important;
+ }
+
+ ul.simple {
+ padding-left: 2rem;
+ }
+ }
+}
A little bit restyling of the top two levels of headings to make them stand out. Again just some personal taste. Unsetting image height in the main article content is critical because otherwise the aspect ratio might be changed, but I'm not sure if this selector is too general (even if limiting only within the article body). The little padding used for dark mode: even if images themselves may not fit well into dark mode at least there's a small padding for transition (because The styling of the Customized buttons+a.btn {
+ &.sk-btn-orange {
+ background-color: var(--sk-orange-tint-1);
+ color: black !important;
+
+ &:hover {
+ background-color: var(--sk-orange-tint-3);
+ }
+ }
+
+ &.sk-btn-cyan {
+ background-color: var(--sk-cyan-shades-2);
+ color: white !important;
+
+ &:hover {
+ background-color: var(--sk-cyan-shades-1);
+ }
+ }
+}
+ color: var(--pst-color-text-base) !important;
+ }
+
+ ul.simple {
+ padding-left: 2rem;
+ }
+ }
+}
Some buttons. The landing page uses both the orange and cyan buttons. The about page uses an orange button. I did not yet find another place where we may want to use this, so not sure if it is proper to put it in the general styling sheet. The colors are chosen such that (1) they are close to the colors originally used on the landing page, and (2) they look well both in light and dark modes. This may also serve as an example of how I intend to use |
Charlie-XIAO
changed the title
Historically, I do not think we ever made the dev landing page unusable. With this PR, the nav bar on the landing page is also gone, so it is a bit harder to navigate to the docs from the landing page. I can see two ways forward:
|
|
@thomasjpfan Thanks for the follow up. I see what you mean.
This is targeting the |
|
Thanks @thomasjpfan for merging. I will make the follow up PRs soon. |
Charlie-XIAO
changed the title
Towards #28084. Please understand that in order to keep this PR as small as possible, it only serves as a foundation and does not really aim at making anything pretty. For instance, the landing page looks absolutely disastrous at this moment, but it is meant to be fixed in some further PR.
conf.py: #28132 (comment)conf.py: #28132 (comment)Here is the list of things left to be done in this PR (if any). Maintainers please feel free to modify.
Ping @glemaitre: this PR is ready for the first round of review.