Use scss nesting & variables #393
Conversation
|
Hello @gnikonorov, you assigned me this ticket. |
Hi @jkowalleck. No action required on your end at all. I assigned you since you're working on it, that's all |
|
i see, @gnikonorov . |
|
@gnikonorov @ssbarnea @BeyondEvil |
There was a problem hiding this comment.
Sorry for falling behind on this on @jkowalleck and thank you for raising! I left a few comments.
|
no worries @gnikonorov |
|
@gnikonorov something seams to fail with this build. i don't see why this is happening with my changes. any advice? |
It is @jkowalleck. The issue is gone now since the PR is merged into master |
gnikonorov
added
code quality
There was a problem hiding this comment.
Thanks for this @jkowalleck! I had one nitpick, but otherwise LGTM. Feel free to ignore my comment if I misunderstood.
@BeyondEvil or @ssbarnea could one of you take a look for a second review/merge?
src/layout/css/style.scss
Outdated
| .log:only-child { | ||
| height: inherit | ||
| } | ||
| $extra-spacing: $spacing; |
There was a problem hiding this comment.
Just curious, why do we need to define $extra-spacing? Can't we just use $spacing?
There was a problem hiding this comment.
sure can use $spacing and remove $extra-spacing.
used $extra-spacing i case the original author had other plans for the spacing in different sections.
now, from a readers perspective i would say it is easier to stay with $spacing.
any opinions from others?
There was a problem hiding this comment.
Remove it please if it's not used @jkowalleck 🙏
* beatify scss * applied variables for fonts, borders, spacing * variables fr exra sizing * add variable for triagle * applied scss nesting
also: scss allowes nesting of css selectors, which was applied here
the CSS was compiled based on changed SCSS.
changes in this PR are related to #383