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

Suggestion: remove font import #34

Closed
montezume opened this issue Nov 29, 2018 · 9 comments
Closed

Suggestion: remove font import #34

montezume opened this issue Nov 29, 2018 · 9 comments
Assignees
@montezume
Labels
released

Comments

@montezume
Copy link
Contributor

montezume commented Nov 29, 2018 •
edited
Loading

ref

It would be nice if it was the responsibility of the consumer of the library to import the required Google Font.

I would suggest leaving out the import, and updating the documentation to reflect the fact that the user should install the font themselves.
@BcRikko
Copy link
Member

BcRikko commented Nov 29, 2018

I was thinking the same thing as you, because I'm a Japanese font user, so Press Start 2P is not enough👍

@IngwiePhoenix
Copy link
Contributor

IngwiePhoenix commented Nov 29, 2018 via email

@montezume
Copy link
Contributor Author

montezume commented Nov 29, 2018 •
edited
Loading

@IngwiePhoenix sure, but then you remove the ability to load fonts async for users that want to do that. And for users that are already loading other Google fonts on their site, they will have to make two requests instead of one.

I think the unintended confusion could be mitigated with decent documentation. Something like:

Usage

This framework depends on the google font Press Start 2P. You can include it on your website like below.

<head>
    <link href="https://fonts.googleapis.com/css?family=Press+Start+2P" rel="stylesheet">
    <link href="path/to/Nes.css" rel="stylesheet">
</head>

@jimmiehansson
Copy link

@IngwiePhoenix sure, but then you remove the ability to load fonts async for users that want to do that. And for users that are already loading other Google fonts on their site, they will have to make two requests instead of one.

I think the unintended confusion could be mitigated with decent documentation. Something like:

Usage

This framework depends on the google font Press Start 2P. You can include it on your website like below.

<head>
    <link href="https://fonts.googleapis.com/css?family=Press+Start+2P" rel="stylesheet">
    <link href="path/to/Nes.css" rel="stylesheet">
</head>

I agree, this makes the documentation more informative and declarative while loosely coupling the distribution of the font from the framework. 👍

@trezy trezy mentioned this issue Nov 30, 2018
Closed
@tuxun
Copy link

tuxun commented Nov 30, 2018 •
edited
Loading

damn it, again google. thx for opening the issue :/

@montezume
Copy link
Contributor Author

I can spin up a PR if there is some consensus here 😄

@IngwiePhoenix
Copy link
Contributor

@montezume I think you may :) Go for it.

@jimmiehansson
Copy link

Looks solid to me 💯

@montezume montezume mentioned this issue Dec 1, 2018
Merged
@BcRikko
Copy link
Member

BcRikko commented Dec 18, 2018

🎉 This issue has been resolved in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

@montezume montezume

Labels
released
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@montezume @IngwiePhoenix @tuxun @jimmiehansson @BcRikko