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

Style: Fix vertical margins of def lists w/o <p>s to match those with #2427

Merged
merged 2 commits into from
Mar 17, 2022

Conversation

CAM-Gerlach
Copy link
Member

@CAM-Gerlach CAM-Gerlach commented Mar 14, 2022

In #2300 , it was reported that the vertical margin varies substantially between entries with multiple paragraphs (i.e, another <p> element) and those that only have one (i.e. just text, no <p>). Furthermore, the margins are still pretty tight as @pradyunsg reported, making them uncomfortable to read; they should be at least equivalent to the normal margin between paragraph-level elements.

PEP 639 is a canonical example without and with this PR.


image


This resolves both of those concerns by just ensuring that dl dd (except headers and footnotes, which are already overridden) have the same 0.5rem bottom vertical margin as paragraphs do:


image


@pradyunsg Are there any additional vertical line spacing concerns you'd like me to address at this time?

Resolves #2300

@CAM-Gerlach CAM-Gerlach added the infra Core infrastructure for building and rendering PEPs label Mar 14, 2022
@CAM-Gerlach CAM-Gerlach requested a review from AA-Turner as a code owner March 14, 2022 03:36
@CAM-Gerlach CAM-Gerlach self-assigned this Mar 14, 2022
@CAM-Gerlach CAM-Gerlach requested a review from hugovk March 14, 2022 03:43
@pradyunsg
Copy link
Member

@pradyunsg Are there any additional vertical line spacing concerns you'd like me to address at this time?

Just one: The line-height set on html should probably change from 1.4rem to just plain 1.4. I don't think I've ever seen someone specify the unit in rem, for line-height. :)

@pradyunsg
Copy link
Member

Actually, looking at PEP 101 again, I'm finding it a bit difficult to specifically point to what exactly looked off to me. I'll file a PR if I'm able to place what it is -- instead of bothering people about the other nebulous whitespace concerns I may have. :)

@CAM-Gerlach
Copy link
Member Author

Seems like this is good to go for now, so I'll go ahead and merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA signed infra Core infrastructure for building and rendering PEPs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix margins of definition lists in the Sphinx theme
4 participants