Skip to content
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

feat: Inter fonts #506

Merged
merged 5 commits into from
Jun 13, 2024
Merged

feat: Inter fonts #506

merged 5 commits into from
Jun 13, 2024

Conversation

kyhyco
Copy link
Contributor

@kyhyco kyhyco commented Jun 10, 2024

What changed? Why?

  • Apply Inter font to doc site

Example

Old New
Screenshot 2024-06-10 at 2 25 00 PM Screenshot 2024-06-13 at 2 39 42 PM

Notes to reviewers
Tried out several approaches:

  • importing @fontsource/inter: messes up the css import path and will require creating a separate package just to handle fonts if we go this route

  • including Inter font files in OnchainKit: this could be an alternative if we don't want to directly hit google fonts

  • using @tailwind/components to create custom font components: much better to create a react component

  • Use case for tailwind and non-tailwind users is supported in this PR

How has it been tested?
locally

@kyhyco kyhyco mentioned this pull request Jun 10, 2024
@kyhyco kyhyco marked this pull request as ready for review June 10, 2024 21:50
@kyhyco kyhyco requested review from Zizzamia and abcrane123 June 10, 2024 21:50
@@ -32,7 +32,9 @@ export default function App({ children }: { children: ReactNode }) {
return (
<WagmiProvider config={wagmiConfig}>
<QueryClientProvider client={queryClient}>
<div style={{ display: 'flex', flexDirection: 'column' }}>{children}</div>
<div style={{ display: 'flex', flexDirection: 'column' }} className="ock-docsite-content">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want all our demo to shocase Inter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question. Let's check in with design tomorrow

@mindapivessa
Copy link
Contributor

@kyhyco I don't think the font applied is Inter?

Here's Inter: https://fonts.google.com/specimen/Inter

Screenshot 2024-06-11 at 12 26 39 PM

@@ -0,0 +1,9 @@
import { ReactNode } from 'react';

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why src/text?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, you are trying to create an internal Design System to use for building all those pieces.

got it!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also do something like src/internal/text

@kyhyco kyhyco changed the title docs: use Inter font for doc site feat: Inter fonts Jun 13, 2024
Copy link
Contributor

@abcrane123 abcrane123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love this !

@Zizzamia Zizzamia merged commit 1cd548a into main Jun 13, 2024
10 checks passed
@Zizzamia Zizzamia deleted the ky.lee/feat/fonts branch June 13, 2024 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants