yous / whiteglass

Minimal, responsive Jekyll theme for hackers
https://yous.github.io/whiteglass/
MIT License
732 stars 201 forks source link

Vertical spacing gone between blocks #50

Closed raygard closed 1 year ago

raygard commented 1 year ago

Summary: calc($spacing-unit / 2) does not work for me; need to replace with calc(#{$spacing-unit} / 2).

I cannot believe this isn't happening to everyone else using whiteglass. Yesterday I updated my blog (raygard.net, aka raygard.github.io) for the first time in a year. Suddenly all the spacing between paragraphs in all posts was gone!

Eventually, I used wget to retrieve my raygard.net/assets/main.css file. In it, I found this: h1,h2,h3,h4,h5,h6,p,blockquote,pre,ul,ol,dl,figure,.highlight{margin-bottom:calc($spacing-unit / 2)}, along with several other instances of calc($spacing-unit / 2). Of course $spacing-unit means nothing in plain CSS.

I know little of CSS and nothing of SCSS or Sass, but I poked around and found that in December there was a commit to change occurrences of $spacing-unit / 2 to calc($spacing-unit / 2), made because of a breaking change to SCSS to deprecate use of / as divide operator outside of calc().

I do not know the rules governing substitution or evaluation of $variables in SCSS, or why it's not working inside calc() for my blog. Maybe the SCSS processor used by github is defective? At the SCSS playground, I tried $foo: 30px; h3{margin-bottom: calc($foo / 2)} and it worked fine, yielding h3{margin-bottom:15px}.

In any case, using interpolation, I find that $foo: 30px; h3{margin-bottom: calc(#{$foo} / 2)} compiles to h3{margin-bottom:calc(30px / 2)} at the playground, and I get similar results in whiteglass by copying _base.css to my blog and changing to calc(#{$spacing-unit} / 2) in _base.scss (also in _layout.scss). So my blog is fixed for now, but I think it needs to be fixed in whiteglass.

yous commented 1 year ago

Thanks for the report. Seeing https://github.com/sass/migrator/pull/198, it seems that calc($spacing-unit / 2) can be replaced with $spacing-unit * 0.5 safely. So I'll update them to use multiplications.

yous commented 1 year ago

I've just released v1.11.3, so please have a check.

raygard commented 1 year ago

Looks good to me. Your fix is much better than mine. (Why didn't I think of * 0.5?) Thanks.