Issue#934 #1565
Issue#934 #1565
Conversation
|
We have tried fixing the following errors: 10:49:28 AM: 12:12 ✖ Expected newline after ":" with a multi-line declaration declaration-colon-newline-after And leave formatting as you had it previously. However, they still appear in our terminal. Plus, we didn't do the npm run publish because we didn´t want to deploy. We were just tring to create unified variables for margins and paddings. Hope this helps you! |
|
@ttkapostol Thanks for the work on this. Those errors are minor but are preventing the site from building a preview. I think it's maybe an issue with character encodings? Or a formatter gone wild. I'll have a fix for this in a bit that hopefully let's the site build. Are you on Windows? |
Thank you very much @davatron5000 ! |
|
@davatron5000 are you going to review this? Else I am happy to review it some time around end of this week. |
|
@SaptakS If you don't mind taking a look. I'll try to review it as well. Sorry this has taken so long. So far it looks good to me. Pretty straight forward. |
|
Will do. |
SaptakS
left a comment
SaptakS
left a comment
There was a problem hiding this comment.
Apologies for the delay in reviewing. This PR looks great! I have left few inline comments on lines I think were maybe mistakenly left from a previous iteration and hasn't been updated? Once those are addressed, I think this PR should be good to be merged.
| @include preserve-list-semantics(); | ||
|
|
||
| margin-top: 1rem; | ||
| margin-top: $m-3; |
| // Text-level formatting | ||
| li { | ||
| margin-top: 0.5rem; | ||
| margin-top: $m-2; |
| .c-homepage-card__description { | ||
| font-family: $font-family-secondary; | ||
| margin-top: 1rem; | ||
| margin-top: $m-3; |
| @include var(color, secondary-link-text); | ||
| font-family: $font-family-secondary; | ||
| margin-top: 1rem; | ||
| margin-top: $m-3; |
|
|
||
|
|
||
| .c-linkroll__category--featured { | ||
| padding-top: 1rem; |
| } | ||
|
|
||
| .u-spacing-left-longest { | ||
| margin-left: 3rem; |
| } | ||
|
|
||
| .u-spacing-bottom-long { | ||
| margin-bottom: 1.5rem; |
|
Ideally, I would also want to reduce the number of variables and maybe normalize some of the spacing to avoid having to need so many different variables |
| $m-9: 4.5rem; | ||
| $m-10: 5rem; | ||
| $m-11: 5.5rem; | ||
| $m-12: 6rem; |
There was a problem hiding this comment.
I was also thinking if we could have something more like:
$m-0: 0;
$m-xxs: 0.5rem;
$m-xs: 1rem;
$m-s: 1.5rem;
$m-m: 2rem;
$m-l: 3rem;
$m-xl: 4rem;
$m-xl: 6rem;
Since it seems like the rest of the variables are never used and changing the naming of the variables removes the need to have every 0.5 step measure defined. @ErriGarcia thoughts?
5 participants
Hello!
We have tried to fix the issue #934 as part of an Open Source Event in Madrid. We have incorporated all repeated measurements in variables. The variables were created according to this table:
Declaration of common measurements for margin and padding
$m-0: 0rem
$m-1: 0.5rem
$m-2: 1rem
$m-3: 1.5rem
$m-4: 2rem
$m-5: 2.5rem
$m-6: 3rem
$m-7: 3.5rem
$m-8: 4rem
$m-9: 4.5rem
$m-10: 5rem
$m-11: 5.5rem
$m-12: 6rem
(Padding is obviously the same but substituting m for p.)
We hope to hear from you soon and collaborate more with you in the future.