SnowdogApps / magento2-theme-blank-sass

SASS based version of Magento 2 Blank theme
MIT License
383 stars 150 forks source link

Remove usage of lib-css mixin and replace false values to null #205

Closed lumnn closed 5 years ago

lumnn commented 5 years ago

lib-css mixin in Less version is used to avoid adding properties when value (or one of them if it's a list) is false.

Sass does have a built in support for removing properties if value is null.

Usage of lib-css makes code much more unreadable and harder to debug with source maps.
To solve this problem I did some work to remove usage of this mixin making code much more simpler.

Changes

Difference in compiled css from master

CSS compiled from master branch, and this PR is slightly different. Here is diff result:

diff -c '--color=always' styles-upstream/styles.css styles/styles.css
*** styles-upstream/styles.css  2018-12-20 14:41:34.029159135 +0100
--- styles/styles.css   2018-12-21 17:38:21.683899239 +0100
***************
*** 3990,3996 ****

  .popup-pointer:after {
    left: 1px;
!   top: 0;
    border: solid 7px;
    border-color: transparent transparent #aeaeae transparent;
    z-index: 98; }
--- 3990,3996 ----

  .popup-pointer:after {
    left: 1px;
!   top: 0px;
    border: solid 7px;
    border-color: transparent transparent #aeaeae transparent;
    z-index: 98; }
***************
*** 5177,5184 ****
    width: 16px;
    background-repeat: no-repeat;
    content: '';
!   display: inline-block;
!   margin: ""; }

  .braintree-paypal-account:before {
    left: 17px;
--- 5177,5183 ----
    width: 16px;
    background-repeat: no-repeat;
    content: '';
!   display: inline-block; }

  .braintree-paypal-account:before {
    left: 17px;

As you can see the only difference is that one property does have 0px instead of 0. Which is a result of calculation in styles/vendor/magento-ui/_utilities.scss linee 398.

Other difference is that one property is not included with empty string. (I guess it's a bug on master)

Backward incompatible changes

Igloczek commented 5 years ago

Sorry for the late response, but I'm not going to merge it. Since version 1.0 of this theme, we decided to keep it just as a port of LESS theme, without any additional improvements, to simplify the maintenance process. We don't want to invest more time into this project, because we are not using it for our clients. If you are looking for a theme with better code quality take a look at Alpaca theme