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 embedded SVG instead of fonts for icons, font-awesome 6.2 #1330

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use embedded SVG instead of fonts for icons, font-awesome 6.2 #1330

wants to merge 1 commit into from

Conversation

notriddle
Copy link
Contributor

@notriddle notriddle commented Sep 15, 2020 •
edited
Loading

The downsides of icon fonts are well-documented, and also, why ship all of the icons when it only uses 14?

Note: font-awesome-as-a-crate 0.2.0 requires Rust 1.49.0, because it performs pattern matching on byte slices in a const fn. See rust-lang/docs.rs#1573

@ehuss
Copy link
Contributor

ehuss commented Sep 18, 2020

Thanks for the PR! I don't think this is something we can land right away, because I believe there are books that are using the FA icons, and this would break them. I'll try to review it when I get a chance, but merging may need to wait until the next major release.

@notriddle
Copy link
Contributor Author

notriddle commented Sep 18, 2020 •
edited
Loading

That’s fine. mdBook is still 0.4, so it can still release a 0.5 some time.

Also, I should probably document the fa handlebars helper in the manual. Since this is how it’s going to be done now. Should we also add some sort of Markdown pass for icons inline with the book?

@notriddle
Copy link
Contributor Author

@ehuss Docs and inline fixup for fontawesome icons have been added to the HTML renderer, so that books using font-awesome will still be able to do so.

@Dylan-DPC-zz Dylan-DPC-zz added the Breaking Change This would require a SemVer breaking change label Mar 11, 2021
@notriddle notriddle force-pushed the embed-svg branch 2 times, most recently from de13cfa to 96c3719 Compare April 14, 2021 22:20
@YJDoc2 YJDoc2 mentioned this pull request Oct 10, 2021
Open
@notriddle notriddle force-pushed the embed-svg branch 2 times, most recently from 3bdc201 to 9b0ccfb Compare December 23, 2021 21:20
@notriddle
Copy link
Contributor Author

@ehuss Now that the fixups are included to allow books that already use font-awesome to continue doing so, and the documentation is written, is there anything else preventing this from being included in the 0.5.0 milestone?

@notriddle notriddle changed the title Use embedded SVG instead of fonts for icons Use embedded SVG instead of fonts for icons, font-awesome 6.2 Sep 18, 2022
@ehuss ehuss added this to the 0.5 milestone Jan 15, 2023
@ehuss ehuss mentioned this pull request Jan 15, 2023
Merged
@KFearsoff KFearsoff mentioned this pull request Dec 6, 2023
Closed
@mgeier mgeier mentioned this pull request May 22, 2024
Open
@notriddle
Use embedded SVG instead of fonts for icons
8763755 
The [downsides of icon fonts] are well-documented, and also, why ship all of
the icons when it only uses 14?

[downsides of icon fonts]: https://speakerdeck.com/ninjanails/death-to-icon-fonts
@uncenter
Copy link

Is there interest in reviving this pull request? I can try to bring these changes up to date with the latest in the repository... 👀

@Dylan-DPC
Copy link
Member

Is there interest in reviving this pull request? I can try to bring these changes up to date with the latest in the repository... 👀

@uncenter yes if you can that would be helpful. Thanks

@uncenter uncenter mentioned this pull request Nov 15, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Breaking Change This would require a SemVer breaking change
Projects
None yet
Milestone
0.5
Development

Successfully merging this pull request may close these issues.
5 participants
@notriddle @ehuss @uncenter @Dylan-DPC @Dylan-DPC-zz