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

FIX: improve display of "side" docstring title #981

Merged
merged 3 commits into from
Oct 6, 2022

Conversation

12rambau
Copy link
Collaborator

@12rambau 12rambau commented Oct 6, 2022

Fix #464

There was already an attempt to solve this issue here:

grid-template-columns: fit-content(30%) minmax(0, 1fr);

We were still loosing a lot of real estate on small screen and for long title there were no space remaining for the content.

In this PR I propose to remove the grid beavior and set the dt tags on top of the dl. It's repsonsive to dark theme and avoid changing the display on small screens:

before:
Capture d’écran 2022-10-06 à 13 53 21

after:
Capture d’écran 2022-10-06 à 13 53 45

visible in: https://pydata-sphinx-theme--981.org.readthedocs.build/en/981/examples/api.html#numpy.linalg.eig

let me know what you think

@12rambau 12rambau marked this pull request as ready for review October 6, 2022 12:05
Copy link
Collaborator

@choldgraf choldgraf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick comment - the docs look good to me!

margin-left: 2rem;
}

dl.field-list {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why move this our of _lists.scss? Wouldn't this also apply to non-API field lists too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this modification was specifically build for API field list so I removed it but if it's set to overwrite the behaviour of every field-list I'm happy to set it back

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the question is whether this would also apply to field lists like in the kitchen sink here:

https://pydata-sphinx-theme.readthedocs.io/en/stable/examples/kitchen-sink/lists.html#field-list

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know we were supporting this kind of behaviour. To me, it looks good as it is.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with whatever. If it also applies to field-lists, but primarily applies to API docs, then let's just add a comment like // Note: this also applies to other field lists but is primarily used for API docs

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait, qui proco here. This modification only applies to API related dt and dl, look at the first very long encapsulating selector, it includes :not(.glossary)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This modification only applies to API related dt and dl, look at the first very long encapsulating selector, it includes :not(.glossary)

Is there a reason not to apply this change to other field lists besides APIs? Consistency seems good to me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APIs generated by numpydocs/napoleon were the only problematic ones. So I would not take the risk to change all the others if they are working nicely

@jarrodmillman jarrodmillman added this to the 0.12 milestone Oct 6, 2022
@drammock drammock merged commit cdc2e8e into pydata:main Oct 6, 2022
@12rambau 12rambau deleted the docstring branch October 6, 2022 14:12
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 this pull request may close these issues.

Mis-render of "side" titles for docstring parameters
4 participants