-
-
Notifications
You must be signed in to change notification settings - Fork 267
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
Replaced all the css into tailwind css on FontSection component #1784
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
.fontsList > li { | ||
margin: 1rem; | ||
} | ||
|
||
.primaryFontFamily div > h6, | ||
.primaryFontFamily div > p { |
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 not find these two classes. @kylemh
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.
1 flaky test on run #4575 ↗︎
Details:
cypress/e2e/modal.spec.js • 1 flaky test • all tests
Review all test suite changes for PR #1784 ↗︎ |
@@ -18,7 +18,7 @@ function FontSection() { | |||
}; | |||
return ( | |||
<li key={item}> | |||
<div> | |||
<div className="[&>p]:font-serif [&>h6]:font-serif"> |
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.
If you do this, the font family will just always be sans-serif... for all font families...
Instead we have to differentiate between primary and secondary AND we have to try rendering the relevant font style. font-serif
is meant to be a fallback.
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 made certain changes,
let me know if that works
|
||
.primaryFontFamily div > h6, | ||
.primaryFontFamily div > p { | ||
font-family: var(--primaryFontFamily), sans-serif; |
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.
Nice job on the condition.
Do you see here how it's 2 values separated by a comma?
- var(--primaryFontFamily)
- sans-serif
That's what's missing.
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.
where to set that variable?
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 the tailwind config
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.
will it look like this ? ->
fontFamily:{
sans:{
primaryFontFamily:'"DIN Condensed 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.
fontFamily: {
primary: "DIN Condensed Bold"
},
and then you can use font-primary
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.
got it
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.
done certain changes
Code Climate has analyzed commit 4143dce and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (90% is the threshold). This pull request will bring the total coverage in the repository to 75.6% (0.0% change). View more on Code Climate. |
Description of changes
Replaced all the css into tailwind css on FontSection component
Issue - #1725
Issue Resolved
Fixes #NA
Screenshots/GIFs