DX-3022 Optimize Sass #864
Conversation
|
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
| display: flex; | ||
| flex-direction: column; | ||
| margin: 2em 0; | ||
|
|
||
| @include mobile-view { | ||
| height: 300px; | ||
| height: 19.8rem; |
There was a problem hiding this comment.
Thinking out loud here... can the style properties that are proportional (height, font-size) be declared as:
$base-font-size: 2.2em;
.base-container{
font-size: $base-font-size;
.something-smaller {
font-size: $base-font-size - 0.2em; or $base-font-size - math.div($base-font-size , 0.11em)
}
}
My syntax may not be accurate but hope it makes sense-:)
There was a problem hiding this comment.
Using Sass would be really beneficial to maintain different components with ease. This is awesome.
Did you end-up using any linter for sass files? If not, may be worth finding one.
Please see if there is any opportunity to pull common properties out as commented.
| } | ||
|
|
||
| h2 { | ||
| font-family: 'Overpass Bold'; |
There was a problem hiding this comment.
What's your opinion on managing common properties like font-family? We're defining this in multiple components, can this be moved to common place and imported where used?
| } | ||
| } | ||
| } | ||
|
|
||
| .right { | ||
| .landing-page-right { | ||
| width: 50%; | ||
| height: 80vh; | ||
| padding-top: 56px; |
There was a problem hiding this comment.
rem is actually not advised for things that are relative to the page size, since the page size is measured in px. For this entire PR I left things like padding, margins, borders, and shadows, which are relative to page size instead of font size, in pixels. That's why it seems like there are a lot of px left over. The only non-page size relative thing I left in px was the amoeba images because the svg measures its size in px and I want to guarantee the aspect ratio stays the same for those
No description provided.