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.

Sign up for GitHub

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

Jump to bottom

feat(design): added Cesko Digital font face #83

Merged
merged 4 commits into from
Oct 31, 2020
Merged

feat(design): added Cesko Digital font face #83

merged 4 commits into from
Oct 31, 2020

Conversation

Erbenos
Copy link

@Erbenos Erbenos commented Oct 28, 2020

No description provided.

@vercel
Copy link

vercel bot commented Oct 28, 2020

Someone is attempting to deploy a commit to the Cesko Digital Team on Vercel.

A member of the Team first needs to authorize it.

@Erbenos Erbenos force-pushed the feat/cesko-digital-font branch from 01d7633 to 961d611 Compare October 28, 2020 20:42
@Erbenos Erbenos mentioned this pull request Oct 28, 2020
Merged
9 tasks
@Erbenos Erbenos force-pushed the feat/cesko-digital-font branch from 961d611 to 559d78d Compare October 28, 2020 21:06
@Erbenos Erbenos force-pushed the feat/cesko-digital-font branch from 559d78d to d2760bf Compare October 28, 2020 21:11
fidelman
fidelman reviewed Oct 29, 2020
View reviewed changes
src/theme/GlobalStyle.js Outdated
${useFont('Cesko Digital Mono', 500, 'fonts/cesko.digital.mono-medium')}
${useFont('Cesko Digital Mono', 600, 'fonts/cesko.digital.mono-semibold')}
${useFont('Cesko Digital Mono', 700, 'fonts/cesko.digital.mono-bold')}

html, body {
Copy link

Choose a reason for hiding this comment

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

pojďme to teď nastavit tady?

Copy link
Author

@Erbenos Erbenos Oct 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Nechápu, font je již nastavený jako výchozí o pár řádků dole.

Copy link

Choose a reason for hiding this comment

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

oh, promin, nevidel jsem

HormCodes
HormCodes suggested changes Oct 29, 2020
View reviewed changes
Copy link
Contributor

@HormCodes HormCodes left a comment

Choose a reason for hiding this comment

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

Vypadá to dobře, díky. 👍

Prosím ale o pár úprav (převážně pozůstatky):

  • GlobalStyle.js převést na global-style.ts a přidat typy
  • default/index.js převést na default.ts

Jinak se mi líbí ten způsob řešení. 🙂 👍

@vercel
Copy link

vercel bot commented Oct 29, 2020 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/ceskodigital/web/8hf112cx9
✅ Preview: https://web-git-feat-cesko-digital-font.ceskodigital.vercel.app

@Erbenos
Copy link
Author

Erbenos commented Oct 29, 2020

Vypadá to dobře, díky. 👍

Prosím ale o pár úprav (převážně pozůstatky):

  • GlobalStyle.js převést na global-style.ts a přidat typy
  • default/index.js převést na default.ts

Jinak se mi líbí ten způsob řešení. 🙂 👍

Můžeme ten index.ts -> default.ts, nechat na poté, co se mergne zbytek theme related commitů, ať nemusím rebasovat všechno?

HormCodes
HormCodes suggested changes Oct 31, 2020
View reviewed changes
Copy link
Contributor

@HormCodes HormCodes left a comment

Choose a reason for hiding this comment

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

Ještě jsem si říkal, zda by byl konflikt i u přejmenování na index.ts, ale dobře. Neříká se mi to lehce, ale můžeme to takto nechat. Poprosím ale o jednu věc.

Chybí nám zde SSR verze pro styled-components, takže font při vypnutém JS se nenačte. Tento plugin by to měl vyřešit. Mohu tě poprosit o vyřešení?

@HormCodes
Copy link
Contributor

@Erbenos, ještě jedna věc. Můžeš přidat bezpatkový fallback font? Přechod je pak celkem znatelný.

@Erbenos
Copy link
Author

Erbenos commented Oct 31, 2020

@Erbenos, ještě jedna věc. Můžeš přidat bezpatkový fallback font? Přechod je pak celkem znatelný.

Fallback již byl nastaven na system-ui a systémové fonty bývají nezpatkové. Problém byl spíše ten, že se pro klienty bez JS nenačetly styly žádné a tudíž ani toto nastavení fontů.... výchozí styly aplikované prohlížeči právě mohou být patkové fonty.

@Erbenos Erbenos force-pushed the feat/cesko-digital-font branch from 878b96e to eb3d7c8 Compare October 31, 2020 17:30
@HormCodes HormCodes merged commit a754bfa into cesko-digital:rework Oct 31, 2020
@Erbenos
Copy link
Author

Erbenos commented Oct 31, 2020 •
edited
Loading

Díky 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fidelman fidelman fidelman left review comments

@HormCodes HormCodes HormCodes requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Erbenos @HormCodes @fidelman