search

HugoGranstrom / nimiSlides

A Reveal.js theme for nimib for making slideshow presentations in Nim
https://hugogranstrom.com/nimiSlides
MIT License
78 stars 5 forks source link

add column-gap parameter #36

Closed quimt closed 4 months ago

quimt commented 8 months ago

parameterized by em units so that a 1.0 gives a reasonably small value independent of screen size

HugoGranstrom commented 8 months ago

Thanks, this seems like a reasonable addition 😄 It seems like you forgot column-gap: in adaptiveColumns. I'll give this a try tomorrow and merge it if I don't find any problems with it 👍

quimt commented 8 months ago

Thanks, this seems like a reasonable addition 😄 It seems like you forgot column-gap: in adaptiveColumns. I'll give this a try tomorrow and merge it if I don't find any problems with it 👍

Indeed it does! Sorry for rushing with the PR, and thanks for considering the merge. I'll try to test the code first before PR, as there are a few more features that might be nice and implementable using raw html or nimib resources upstream.

HugoGranstrom commented 8 months ago

Oh, optional arguments doesn't work with body: untyped sadly. So you have to create a template without the new parameter:

template columns(body: untyped) =
  columns(0, body)
quimt commented 8 months ago

Oh, optional arguments doesn't work with body: untyped sadly. So you have to create a template without the new parameter:

template columns(body: untyped) =
  columns(0, body)

Merged the change after testing. Templates also don't integrate nicely with fmt from std/strformat, so I've used & instead.

HugoGranstrom commented 8 months ago

Nice, thanks! 😁 I didn't get the free time today that I wanted so I haven't had time to test this, and I won't have the time for the rest of the week either sadly (I'm moving, lots of packing to be done). But I will get back to you next week (and if I don't, ping me).

quimt commented 7 months ago

Ping!

And thanks for filling out this part of the documentation/outreach pipeline for Nim!

quimt commented 7 months ago

Implemented now. 0.0 was consistent with preexisting code in chrome; this led to crowding which was an extra incentive for the change. I agree that 1em or 0.5em is a more sensible default.

HugoGranstrom commented 7 months ago

Thanks, weird that the default was 0 when they say it should be 1 🤔

There are still 2 comments that you have to address before I can merge this though. As it is now, the code doesn't work if you have multiple columns on the same slides.

quimt commented 7 months ago

Fixed the overdeletion bugs of the </div> and column-gap .

image

Changed to strutils %-interpolation to match style of surrounding code.

quimt commented 4 months ago

Sorry, missed the email in the end-of-year hustle. The fix is complete.

quimt commented 4 months ago

...or it should have been complete. Sorry about the oversight about interpolation via %. I PR'd one more time.

HugoGranstrom commented 4 months ago

Nice job! Thanks, I'll have a final look in the evening but it should be done now 🎉