pietroppeter / nimib

nimib 🐳 - nim 👑 driven ⛵ publishing ✍
https://pietroppeter.github.io/nimib/
MIT License
175 stars 10 forks source link

reduced vertical spacing in default theme #133

Closed pietroppeter closed 1 year ago

pietroppeter commented 1 year ago

this was stimulated from this particular example, where I realized that - at least for terminal output - the vertical spacing is not ideal.

before: image

after: image

in the current fix instead of just fixing the terminal output the vertical spacing is reduced everywhere (in a body css selector in used in default theme). Not sure if this is welcome or not (it helps packing more content vertically, it might decrease readability). I want to have a look on how this would look in nimib documentation and then decide if I want to restrict it only on terminal output (but I would need to learn the css selector that makes it work).

as a reminder, when this is ready it will need a:

pietroppeter commented 1 year ago

Nope, this does not work. Just by looking from mobile, it is definitely too packed:

image

HugoGranstrom commented 1 year ago

CSS is so dumb sometimes. If I tried changing the line-height of the samp tag, it did nothing. But adding it to the pre tag seems to work. The problem is that it affects all pre tags, including the one around code blocks.

There are two solutions in my mind:

  1. Create a output-pre class and assign it to the pre tag.
    output-pre {
    line-height: 1.15;
    }
    --------
    doc.partials["nbCodeOutput"] = """{{#output}}<pre class="output-pre"><samp>{{output}}</samp></pre>{{/output}}"""
  2. Inline the style directly in the mustache template
    doc.partials["nbCodeOutput"] = """{{#output}}<pre style="line-height: 1.15;"><samp>{{output}}</samp></pre>{{/output}}"""
pietroppeter commented 1 year ago

Thanks for looking into this. Found a third option, that should work (have not tested it): using pre:has(> samp) css selector, see:

But this is used currently on 56% of users.

So I would go with one of your two options: the class seems cleaner (and more reusable) but it can break more easily since there is code in two places (eg in a new theme block output will likely not be customized but you are likely not using the nbCodeStyle). Still I think I would go with the class option.

HugoGranstrom commented 1 year ago

Thanks for looking into this. Found a third option, that should work (have not tested it): using pre:has(> samp) css selector, see:

I found it as well, but given that I use Firefox both on my laptop and on my phone, it fell away quite quickly :upside_down_face:

So I would go with one of your two options: the class seems cleaner (and more reusable) but it can break more easily since there is code in two places (eg in a new theme block output will likely not be customized but you are likely not using the nbCodeStyle). Still I think I would go with the class option.

Sounds good to me :+1: I'll have to manually fix it either way in nimiSlides no matter how we do it.

pietroppeter commented 1 year ago

implemented your class solution (naming the class nb-output). let's see how it looks in the preview...

pietroppeter commented 1 year ago

example of before/after taken from penguins:

image

looks fine to me. Thoughts before I merge?

HugoGranstrom commented 1 year ago

The left one looks better to me, so hope that is the new one :rofl: :speak_no_evil: (seems like it is, then go ahead!)

pietroppeter commented 1 year ago

Haha yes, it is indeed an after/before