search

useblocks / sphinx-simplepdf

A simple PDF builder for Sphinx documentations
https://sphinx-simplepdf.readthedocs.io
MIT License
36 stars 15 forks source link

[#34] fix overlap content in tables with extra class option #35

Closed kreuzberger closed 1 year ago

kreuzberger commented 1 year ago

This fix adds an extra class option in the css to allow html output to break in words to render content. This solution allows to fine tune the behaviour for individual tables and respects given colwidths in needs table directives.

Please feel free to improve css handling/naming/subclassing, i am no expert in css.

danwos commented 1 year ago

Thanks for the PR :+1:

Overall it all looks good, but I'm thinking about using some kind of a namespace for our CSS-configs provided by Sphinx-SimplePDF (ssp). Something like ssp-table-wrap.

The chance is high, that there is one theme or a project-specific CSS-config, which is already using this name. It may affect the PDF output or our set class affects the HTML theme.

What do you think about adding ssp- to all our upcoming CSS-classes?

Ohh, and please add also a small hint to the docs somewhere and a one-liner to changelog.rst. Thanks :)

kreuzberger commented 1 year ago

Two questions: Namespacing is ok, What stands sn- for? Second: Not all needs-directives support class option. except needs-import that allow only styles. Should _needs.scss be adapted for a style "needs_style_ssp-table-wrap" or not?

danwos commented 1 year ago

Argh sorry, I mean ssp- (corrected) and not sn- (I use sn as a synonym for Sphinx-Needs.)

Second question: good point. I think your proposal is the only way to go, if a generic solution for all needs is not feasible. Maybe in the table-wrap-case it makes sense to set it automatically for all needs, I see no use case where overflow-wrap: break-word; makes no sense for a need.

kreuzberger commented 1 year ago

updated. For "wider" tables, which are intend to be wide (e.g. the large grid table in demo) i added also an option to change layout to landscape inbetween the pdf. Must update changelog.rst :-)

danwos commented 1 year ago

Looks all good. The only thing I'm missing is some docs under docs/ :) No need to write much, but at least the new classes should be mentioned somewhere and a one-liner as a description.