-
-
Notifications
You must be signed in to change notification settings - Fork 11
830-refactor: Move style params to scss variables #891
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
d1635ce
dbdb1b5
c8f50fe
79e6785
cb96879
13fc612
edb65aa
7663180
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,5 +43,5 @@ | |
} | ||
|
||
.subresult p { | ||
font-size: 14px; | ||
font-size: $font-size-xs; | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -95,3 +95,28 @@ $ease-standard-accelerate: cubic-bezier(0.3, 0, 1, 1); | |||||
$ease-emphasize-decelerate: cubic-bezier(0, 0, 0, 1); | ||||||
$ease-in-cubic: cubic-bezier(0.32, 0, 0.67, 0); | ||||||
$standard-decelerate: cubic-bezier(0, 0, 0, 1); | ||||||
|
||||||
// Border radius | ||||||
$border-radius-xs: 8px; | ||||||
$border-radius-s: 12px; | ||||||
$border-radius-m: 16px; | ||||||
$border-radius-l: 20px; | ||||||
$border-radius-xxxl: 999px; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest to change it to |
||||||
|
||||||
// Font sizes | ||||||
$font-size-xxs: 12px; | ||||||
$font-size-xs: 14px; | ||||||
$font-size-s: 16px; | ||||||
$font-size-m: 18px; | ||||||
$font-size-l: 20px; | ||||||
$font-size-xl: 24px; | ||||||
$font-size-xxl: 36px; | ||||||
|
||||||
// Gap | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
$gap-xs: 8px; | ||||||
$gap-s: 16px; | ||||||
$gap-m: 20px; | ||||||
$gap-l: 24px; | ||||||
$gap-xl: 32px; | ||||||
$gap-xxl: 40px; | ||||||
$gap-xxxl: 50px; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,7 @@ | |
.course-info { | ||
display: flex; | ||
flex-direction: column; | ||
gap: 8px; | ||
gap: $gap-xs; | ||
|
||
width: 100%; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, add
$border-radius-xxs: 4px;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it. But I'm not sure about the names of z-indexes though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our project we have
-1
,1
,2
,10/100
. Maybe:-1
-bottom
,1
-default
,2
-top
,10/100
-max
@SpaNb4 @Quiddlee what do you think, guys?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ansivgit, I think we could use something similar to:

https://www.smashingmagazine.com/2021/02/css-z-index-large-projects/
As alternatives to
bottom
andtop
Also, shouldn't the default be 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it! And I just suggest to add
max = 999