-
Notifications
You must be signed in to change notification settings - Fork 17
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
13 trucks carousel #82
Conversation
Hello, I'm Franklin Bot and I will run some test suites that validate the page speed.
|
|
|
|
|
…into 13-trucks-carousel
|
|
|
} | ||
|
||
/* stylelint-disable-next-line no-descending-specificity */ | ||
.v2-truck-lineup__navigation-item button { |
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.
Could you reset the font-weight
here to get rid of faux bold?
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.
Instead of changing this button, I updated in styles, because by default button is using font-family Helvetica Roman, so the weight there was already wrong.
|
||
.v2-truck-lineup__navigation-item.active button, | ||
.v2-truck-lineup__navigation-item button:hover { | ||
--color-icon: var(--c-primary-black); |
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.
The icon should be getting the Accent/Copper colour here
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.
Just the icon is using accent copper color, can you confirm? And check if the behaviour is how you expect?
|
|
…into 13-trucks-carousel
|
|
|
Usually the titles are done in another component, can you confirm @cogniSyb @Lakshmishri that in Mack we follow the same? |
We should definitely pick this up and put it in a separate block :) |
Fix #13
Test URLs: