add column-gap parameter

[quimt]

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

[HugoGranstrom]

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]

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]

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]

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]

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]

Ping!

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

[HugoGranstrom]

Thanks for the reminder 😁 I will try the code out tonight but left a comment about string interpolation.

Glad that our work is appreciated ❤️

[HugoGranstrom]

I've now been able to test the code out locally and with the requested changes it works for me. So once you have adressed them I'll gladly merge your contributions :D

[quimt]

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]

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]

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

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

[HugoGranstrom]

Thank you 😁 there seems to have snuck in one more typo, but after that I think I'm content 😄 I will give it a run when I get home in the evening

[quimt]

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

[quimt]

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

[HugoGranstrom]

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

[HugoGranstrom]

Looks good, thank you again for you contribution 😁 This will make it in the next release of nimiSlides