jekyll / minima

Minima is a one-size-fits-all Jekyll theme for writers.
https://jekyll.github.io/minima/
MIT License
3.4k stars 3.62k forks source link

Fix deprecation warnings (#796) #797

Closed KarlHeitmann closed 1 month ago

KarlHeitmann commented 1 month ago

Description

I've cloned this repo on my machine, after running bundle install, I executed the ./script/server command and I've got the same deprecation message described by me in #796 . The only difference is I used the next patch version of ruby: ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-linux] in my local minima repo.

The cause of the error seems to be the SASS compiler (I guess this version I have in my Gemfile.lock: sass-embedded (1.77.8-x86_64-linux-gnu)), because the deprecation message told me to read this page: https://sass-lang.com/documentation/breaking-changes/mixed-decls/

The deprecation message warns the offending line can impact the appearance of a page in the browser when the CSS nesting module will be introduced in the future https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_nesting

Proposal

After reading the deprecation explanation, I took the decision to opt-out by moving both offenders before the nested rule.

Why? Because the caveat / edge case the CSS Working Group took in consideration on this issue: https://github.com/w3c/csswg-drafts/issues/8738 and the example found on the main sass-lang web page linked above are completely different than the case we have in minima theme.

Note both code snippets have the same property assigned a different value inside the nested rule, and the line below the nested rule.

In our case, the property found below the nested rule is margin-left, and the property found inside the nested rule is margin-right. So I opted out, instead of opting in by using the & selector.

I don't have a strong opinion here, if you want to opt in, I can change the PR. But I've tested the output on my local host by visually inspecting and using the dev tools to check the right CSS rules are applying correctly by looking at the a.page-link element found in _includes/header.html on my browser.

Everything looks good with the changes I've made.

I'll copy the content of my Gemfile.lock file below:

Gemfile.lock ``` PATH remote: . specs: minima (3.0.0.dev) jekyll (>= 3.5, < 5.0) jekyll-feed (~> 0.9) jekyll-seo-tag (~> 2.1) GEM remote: https://rubygems.org/ specs: addressable (2.8.7) public_suffix (>= 2.0.2, < 7.0) bigdecimal (3.1.8) colorator (1.1.0) concurrent-ruby (1.3.3) em-websocket (0.5.3) eventmachine (>= 0.12.9) http_parser.rb (~> 0) eventmachine (1.2.7) ffi (1.17.0-x86_64-linux-gnu) forwardable-extended (2.6.0) google-protobuf (4.27.3-x86_64-linux) bigdecimal rake (>= 13) http_parser.rb (0.8.0) i18n (1.14.5) concurrent-ruby (~> 1.0) jekyll (4.3.3) addressable (~> 2.4) colorator (~> 1.0) em-websocket (~> 0.5) i18n (~> 1.0) jekyll-sass-converter (>= 2.0, < 4.0) jekyll-watch (~> 2.0) kramdown (~> 2.3, >= 2.3.1) kramdown-parser-gfm (~> 1.0) liquid (~> 4.0) mercenary (>= 0.3.6, < 0.5) pathutil (~> 0.9) rouge (>= 3.0, < 5.0) safe_yaml (~> 1.0) terminal-table (>= 1.8, < 4.0) webrick (~> 1.7) jekyll-feed (0.17.0) jekyll (>= 3.7, < 5.0) jekyll-sass-converter (3.0.0) sass-embedded (~> 1.54) jekyll-seo-tag (2.8.0) jekyll (>= 3.8, < 5.0) jekyll-watch (2.2.1) listen (~> 3.0) kramdown (2.4.0) rexml kramdown-parser-gfm (1.1.0) kramdown (~> 2.0) liquid (4.0.4) listen (3.9.0) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) mercenary (0.4.0) pathutil (0.16.2) forwardable-extended (~> 2.6) public_suffix (6.0.1) rake (13.2.1) rb-fsevent (0.11.2) rb-inotify (0.11.1) ffi (~> 1.0) rexml (3.3.4) strscan rouge (4.3.0) safe_yaml (1.0.5) sass-embedded (1.77.8-x86_64-linux-gnu) google-protobuf (~> 4.26) strscan (3.1.0) terminal-table (3.0.2) unicode-display_width (>= 1.1.1, < 3) unicode-display_width (2.5.0) webrick (1.8.1) PLATFORMS x86_64-linux DEPENDENCIES bundler minima! BUNDLED WITH 2.4.20 ```
ashmaroli commented 1 month ago

Thanks @KarlHeitmann @jekyllbot: merge +fix