Skip to content
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

Mis-render of "side" titles for docstring parameters #464

Closed
bryevdv opened this issue Sep 6, 2021 · 13 comments · Fixed by #981
Closed

Mis-render of "side" titles for docstring parameters #464

bryevdv opened this issue Sep 6, 2021 · 13 comments · Fixed by #981
Assignees

Comments

@bryevdv
Copy link
Contributor

bryevdv commented Sep 6, 2021

cc @choldgraf @jorisvandenbossche

As the viewport narrows:

Screen Shot 2021-09-05 at 18 51 03

Leading eventually to:

Screen Shot 2021-09-05 at 18 51 12

I would personally like to see the "Parameters" and "Keyword Arguments" headings above their respective lists, rather than off to the side, with the lists of parameters and their docs getting full (or nearly full) width, Apart from the amusing results above on narrow viewports, it's also a huge waste of vertical space below the headings on wide viewports:

Screen Shot 2021-09-05 at 18 55 33

@drammock
Copy link
Collaborator

drammock commented Sep 7, 2021

we (MNE-Python) fixed the wrapping problem in our local CSS like this:

dl.field-list {
    grid-template-columns: auto 1fr;
}

I can put in a PR if that seems an acceptable solution for the general case.

@bryevdv
Copy link
Contributor Author

bryevdv commented Sep 7, 2021

Not sure what is different about our setup but that change does not seem to have much or any effect (verified the stylesheet loaded in the browser has the change)

@drammock
Copy link
Collaborator

drammock commented Sep 7, 2021

oops, you're right, sorry. I remembered dealing with this issue, and I naively checked the browser console to see what rules from our custom CSS were applying to that node or its parent. But if I look at the PR where I actually tackled the problem my past self says that I expressly didn't fix the wrapping problem, which probably needs to be addressed by adjusting the media breakpoints in the theme (?).

@drammock
Copy link
Collaborator

drammock commented Sep 7, 2021

...although, strangely, it doesn't seem to happen on the MNE-Python site, so maybe I did eventually fix it in a different PR? Will dig deeper.

@choldgraf
Copy link
Collaborator

I'm totally +1 on fixing this, but not sure what to do. IF somebody figures out a CSS fix for it I am happy to review a PR!

@choldgraf
Copy link
Collaborator

@bryevdv it would be helpful if you provided a link to a page with this problem

@bryevdv
Copy link
Contributor Author

bryevdv commented Sep 7, 2021

@choldgraf https://docs.bokeh.org/en/2.3.3/docs/reference/plotting.html#bokeh.plotting.Figure.bezier

Just reiterating that personally I think a structural change that splits each args section into its own separate dl might be preferable (again I'd like to see those as headers above each list, not off to the side, taking up huge columns of empty space below)

@drammock
Copy link
Collaborator

drammock commented Sep 7, 2021

I can reproduce the problem @bryevdv is seeing, and I confirm that my suggested CSS rule doesn't fix it on the Bokeh site. I don't have time right now to rigorously compare the CSS files and conf.pys to find what the source of the different behavior might be.

Just reiterating that personally I think a structural change that splits each args section into its own separate dl might be preferable

in some quick-but-not-rigorous testing, this is possible with:

dl.field-list {
    display: unset;
}

or

dl.field-list {
    display: grid;
    grid-template-columns: unset;

If the theme maintainers want to go that route, however, I'd expect some corresponding style changes to the dt elements and/or a small indent on the dd elements, to make the dts stand out more as heading-like things and the dds as more content-like things.

@choldgraf
Copy link
Collaborator

this is an autodoc specific thing right? I am happy to special-case autodoc output with some CSS that only works for it, or something like that

@bryevdv
Copy link
Contributor Author

bryevdv commented Sep 8, 2021

@choldgraf we use Google style docstrings with sphinx.ext.napoleon It is supposed to outputs ReST like

:param arg1: Description of `arg1`
:type arg1:

by default. AFAIK those are processed by the sphinx std python domain.

@bryevdv
Copy link
Contributor Author

bryevdv commented Sep 8, 2021

@drammock yah that could use some tweaking but it's already much better, I will probably put this with some tweaks in the next Bokeh release as a stopgap, just because of how much it improves (looks good on narrow viewport too). Thank you!

Screen Shot 2021-09-07 at 21 13 52

@jorisvandenbossche
Copy link
Member

The whole docstring / API page styling can probably use a redesign (there were some other autodoc related changes due to sphinx as well: #460)

I agree that having the "Parameters" etc in a separate column is not very robust long term, it will always give problems when the screen gets smaller (in addition to wasting space).
This is actually inherited from the basic.css which sets dl.field-list to display: grid;

As usual, we can get some inspiration from Furo: https://pradyunsg.me/furo/reference/api/. It puts the title above the content (not inheriting from basic.css gives that automatically), and adds a small indentation to the content (and in addition also makes the title upper case). CSS: https://github.com/pradyunsg/furo/blob/main/src/furo/assets/styles/content/_api.sass

@choldgraf
Copy link
Collaborator

+1 to doing whatever Furo does :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants