-
-
Notifications
You must be signed in to change notification settings - Fork 89
🐛 Fix: Apply fonts to books and 💄 dropdown boxes render the font #721
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
Conversation
|
Okay documentation has been updated |
aaronleopold
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thank you!
I don't know if this supersedes it (I think it does) [the other PR]
I think it does, at the very least I can see the fonts being requested by the component locally and they seem to be working as expected. This is also a good bit leaner of a change, which is a plus.
I did not fix the flashing when changing line height / font / font size
I think this is an artifact of epub.js, so I think it's acceptable
I made sure the dropdown boxes for font in 'reader settings' rendered in their own font
I appreciate this!
| const injectFontStylesheet = (rendition: Rendition) => { | ||
| const doc = Object.values(rendition.getContents())[0]?.document | ||
| if (!doc) return | ||
|
|
||
| const head = doc.head | ||
| if (!head) return | ||
|
|
||
| const link = doc.createElement('link') | ||
| link.rel = 'stylesheet' | ||
| link.id = 'stump-fonts-stylesheet' | ||
| link.href = '/assets/fonts/fonts.css' | ||
| head.appendChild(link) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
Oh I see. I was testing out an implementation of foliate-js because of my Japanese book issues and it didn't flicker. foliate-js.mov |
My browser doesn't want to show me that video for whatever reason, but I'll trust you 😅 I haven't explored foliate-js, epub.js has been a headache to work with in the past. The mobile app will likely use readium since they have native toolkits, so I had loose plans to explore that for web at some point. If it's an area of interest you're welcome to try and port the reader to either, at the end of the day if it can support highlights/annotations, epubcfi-based progress tracking, etc, and provides a better experience, it's a win I would ask that you base it into |
I moved the fonts, and fixed typos inside the font css files, so the real changes were in the 6 files:
I know there's another pull request for this fix #663, and I'm not sure what other features that one included so I don't know if this supersedes it (I think it does). I did try that fork but there seemed to be issues like colour of text and background not loading correctly, etc. Solves #660 I guess.
I did not fix the flashing when changing line height / font / font size.
I made sure the dropdown boxes for font in 'reader settings' rendered in their own font and not the font set under 'appearance'.
Edit: I didn't update the documentation either. I'll do that now and send an update.