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

Use fantasticon to generate web fonts #515

Merged
merged 5 commits into from
Dec 9, 2020
Merged

Use fantasticon to generate web fonts #515

merged 5 commits into from
Dec 9, 2020

Conversation

mdo
Copy link
Member

@mdo mdo commented Dec 7, 2020 •
edited
Loading

Replaces #287 with a new dependency, fantasticon. Fixes #86.

@mdo mdo added the enhancement New feature or request label Dec 7, 2020
@mdo mdo mentioned this pull request Dec 7, 2020
Closed
@mdo mdo force-pushed the webfont branch 2 times, most recently from 35446d3 to 4b072da Compare December 7, 2020 23:00
@mdo mdo marked this pull request as ready for review December 8, 2020 05:06
XhmikosR
XhmikosR reviewed Dec 8, 2020
View reviewed changes
.fantasticonrc.js Outdated Show resolved Hide resolved
XhmikosR
XhmikosR reviewed Dec 8, 2020
View reviewed changes
src/html.hbs Outdated Show resolved Hide resolved
XhmikosR
XhmikosR reviewed Dec 8, 2020
View reviewed changes
package.json Show resolved Hide resolved
XhmikosR
XhmikosR requested changes Dec 8, 2020
View reviewed changes
Copy link
Member

@XhmikosR XhmikosR left a comment

Choose a reason for hiding this comment

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

Also,

  • the hbs files seem unused
  • package.json is missing the new devDependency
  • the script should be added to another script (or we run it separately) so that's tested on CI

@mdo
Copy link
Member Author

mdo commented Dec 8, 2020

  • the hbs files seem unused

.hbs files are indeed used—they help generate the CSS and HTML in the font directory. They're specified in .fantasticonrc.js.

@mdo
Copy link
Member Author

mdo commented Dec 8, 2020

  • package.json is missing the new devDependency
  • the script should be added to another script (or we run it separately) so that's tested on CI

Done and done.

  • Hooked into the icons script
  • Added devDependency
  • Renamed the font script to icons-font for consistency
  • Added the font dir to the zip script

@mdo
Generate webfonts via fantasticon
dfc62ee 
- Includes new docs section and fonts page
- Generates CSS and JSON in addition to 4x font types
- Adds new script and hooks into existing icons and zip scripts
@XhmikosR
Copy link
Member

XhmikosR commented Dec 8, 2020

Just a question: do you really want to add eot and ttf fonts? woff and woff2 should be more than enough. Heck, you could even just generate woff2 if you don't care about IE :P

XhmikosR
XhmikosR reviewed Dec 8, 2020
View reviewed changes
src/html.hbs Outdated Show resolved Hide resolved
src/html.hbs Show resolved Hide resolved
@mdo
Copy link
Member Author

mdo commented Dec 9, 2020

Dropping EOT and TTF for now since we can always add back later, but removing is always more difficult.

Good to go after this I think!

@mdo mdo requested a review from XhmikosR December 9, 2020 03:59
@XhmikosR
Copy link
Member

XhmikosR commented Dec 9, 2020

Minus my last comment, I think this looks good :)

@XhmikosR
Copy link
Member

XhmikosR commented Dec 9, 2020

I wonder though, is fonts/index.html supposed to be served? It's not using relative paths to the CSS AFAICT.

@mdo
Copy link
Member Author

mdo commented Dec 9, 2020

I wonder though, is fonts/index.html supposed to be served? It's not using relative paths to the CSS AFAICT.

Yeah, I want that served. It's used locally for previewing, and eventually I'll have it up to date and part of the full nav.

@XhmikosR
Copy link
Member

XhmikosR commented Dec 9, 2020 •
edited
Loading

Actually, I just saw you hooked the fonts directory as fonts so it's already available indeed: https://deploy-preview-515--bootstrap-icons.netlify.app/font/ and included in sitemap.

@mdo mdo merged commit 03d6dae into main Dec 9, 2020
@mdo mdo deleted the webfont branch December 9, 2020 19:27
@ffoodd ffoodd mentioned this pull request Dec 10, 2020
Open
6 tasks
@TechQuery
Copy link

TechQuery commented Dec 12, 2020 •
edited
Loading

@mdo @XhmikosR There is no CSS & Web Font files in the ZIP bundle & NPM package...

@XhmikosR
Copy link
Member

The zip file does have the font for me. For the npm package see #538 .

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

@XhmikosR XhmikosR XhmikosR approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide font-family format
3 participants
@mdo @XhmikosR @TechQuery