optimize figure bottom margin

pydata / pydata-sphinx-theme

A clean, three-column Sphinx theme with Bootstrap for the PyData community

https://pydata-sphinx-theme.readthedocs.io

BSD 3-Clause "New" or "Revised" License

600 stars 313 forks source link

optimize figure bottom margin #1749

Closed berlin2123 closed 5 months ago

berlin2123 commented 6 months ago

Avoid multiple margin-bottom by p and figure, that result in too wide a bottom margin.

trallard commented 6 months ago

Thanks for the contribution @berlin2123, since there is no associated issue with this PR could you please add a screenshot of the before and after this change so we understand the rationale behind it?

berlin2123 commented 6 months ago

before:

5(6G_$L5D~2BTVR$4JV@KR4

after:

_WF9TF%LGK%`}B%DZ(14_NG

drammock commented 5 months ago

seems like a reasonable change to make. Linter is failing; @berlin2123 can you try installing our pre-commit hook? It should auto-fix linter errors at commit time.

trallard commented 5 months ago

@gabalafou This seems reasonable, but a last check would be helpful. If you are happy with this and so is the CI, you can go ahead and merge.

gabalafou commented 5 months ago

I pushed a few tweaks:

I added an example to Images and Figures that demonstrates the changes in this PR
I moved the margin-top adjustment to the figcaption because I think that's where it more properly belongs. I believe the intent is to just add some whitespace between the image and the caption, not add more whitespace between every paragraph in the caption
Similarly I moved the margin-bottom adjustment to the last paragraph in the figcaption, since I don't think the intent was to reduce the margin between paragraphs but only to prevent the last paragraph's bottom margin from adding on to the figcaption's bottom margin

gabalafou commented 5 months ago

Before (figure example section)

pst-figures-example-before

After (figure example section)

pst-figures-example-after

gabalafou commented 5 months ago

@drammock could you give this another round of review?

gabalafou commented 5 months ago

@drammock, ah, good to know. I decided to remove the example because it's essentially already covered at the bottom of Kitchen Sink / Structural Elements.

That said, I think the figure section should probably also have an example like the one on the structural elements page. Would it be possible to make this change upstream?

drammock commented 5 months ago

Would it be possible to make this change upstream?

you'd have to propose the change at https://github.com/sphinx-themes/sphinx-themes.org

If you clearly explain the rationale for the change (as you typically do 👍🏻) I'm confident that @pradyunsg would merge it