Solutions Block #63 #95
Conversation
markovukiceviccn
requested review from
kesiah,
Lakshmishri,
TomaszDziezykNetcentric and
cogniSyb
|
Hello, I'm the AEM Code Sync Bot and I will run some test suites that validate the page speed.
|
|
blocks/v2-solutions/v2-solutions.css
Outdated
| font: normal var(--body-font-size-s)/1.5 var(--ff-body); | ||
| } | ||
|
|
||
| .v2-solutions > div > div > p:first-child { |
There was a problem hiding this comment.
This should be a shared style. Can you reuse code what’s in the Volvo repository? It would replace current code in another block as well.
There was a problem hiding this comment.
This is now changed and use adjustPretitle function.
There was a problem hiding this comment.
Could you then refactor v2-columns.js as well? When the pretitle styling is put in styles.css, then code could be removed from v2-columns.css
blocks/v2-solutions/v2-solutions.css
Outdated
| display: flex; | ||
| flex-direction: column; | ||
|
|
||
| @media (min-width: 744px) { |
There was a problem hiding this comment.
Could you refactor the CSS nesting? There’s not enough support for it in Firefox.
There was a problem hiding this comment.
Media queries nesting adjusted.
blocks/v2-solutions/v2-solutions.css
Outdated
| } | ||
|
|
||
| .v2-solutions .button.button--primary { | ||
| font-family: var(--ff-subheadings-medium); |
There was a problem hiding this comment.
Remove, and please fix https://github.com/Netcentric/vg-macktrucks-com-rd/blob/main/styles/styles.css#L868 instead :)
There was a problem hiding this comment.
Font family is now changed.
There was a problem hiding this comment.
I meant changing the standard font-family for buttons in styles.css (for redesign)
There was a problem hiding this comment.
This is now done
|
blocks/v2-solutions/v2-solutions.css
Outdated
| font: normal var(--body-font-size-s)/1.5 var(--ff-body); | ||
| } | ||
|
|
||
| .v2-solutions .pretitle { |
There was a problem hiding this comment.
Could you move this to styles.css? Instead of .v2-solutions prefix .v2-redesign could be used. Hope the specificity is high enough :)
There was a problem hiding this comment.
This is now done.
blocks/v2-solutions/v2-solutions.css
Outdated
| font: normal var(--body-font-size-s)/1.5 var(--ff-body); | ||
| } | ||
|
|
||
| .v2-solutions > div > div > p:first-child { |
There was a problem hiding this comment.
Could you then refactor v2-columns.js as well? When the pretitle styling is put in styles.css, then code could be removed from v2-columns.css
blocks/v2-solutions/v2-solutions.css
Outdated
| flex-direction: column; | ||
| } | ||
|
|
||
| @media (min-width: 744px) { |
There was a problem hiding this comment.
Now that it’s not nested anymore, could you group the breakpoints together at the bottom of the CSS? Then the code structure follows the majority of the code base
There was a problem hiding this comment.
It is now grouped.
|
Fix #63
Test URLs:
Block background colour will be delivered in this ticket:
#64