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

Add font size and theme selection controls to EPUB reader #4285

Merged
merged 10 commits into from
Mar 28, 2023
Merged

Add font size and theme selection controls to EPUB reader #4285

merged 10 commits into from
Mar 28, 2023

Conversation

LokiLuciferase
Copy link
Contributor

This PR adds some basic theming controls to the built in EPUB reader.

I'm not a JS dev and I mainly implemented this for myself so I can finally ditch Google Play Books, so while I am happy to try and implement any requested changes, do feel free to reject this PR without further comment if the code quality is too bad to salvage.

Changes

  • update white-on-black font color to be slightly off-white (more pleasant to view IMHO)
  • add decrease and increase font size buttons
  • add button which cycles through bright-on-black, sepia-on-dark (similar to Google Play Books) and black-on-bright themes

Issues

Screenshots
default
largerfont
sepia

@sonarcloud
Copy link

sonarcloud bot commented Jan 15, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Jan 20, 2023
@thornbill thornbill added the feature New feature or request label Feb 6, 2023
thornbill
thornbill requested changes Mar 7, 2023
View reviewed changes
Copy link
Member

@thornbill thornbill left a comment

Choose a reason for hiding this comment

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

Hey @LokiLuciferase would you be able to fix the merge conflicts on this? Thanks!

@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Mar 8, 2023
@LokiLuciferase
Copy link
Contributor Author

Hi @thornbill, I rebased my changes on top of master, should be fine now!

@LokiLuciferase
Copy link
Contributor Author

NB: I just realized there was a flaw in my previous implementation: The theme selection button did not cycle properly, but instead after having selected all available themes once, it did not return to the original theme unless one closes and re-opens the window. Seems like a limitation of epubjs. This is fixed in the latest commit by updating the theme instead of registering multiple themes and cycling through them.

thornbill
thornbill requested changes Mar 9, 2023
View reviewed changes
src/plugins/bookPlayer/plugin.js Outdated Show resolved Hide resolved
src/plugins/bookPlayer/plugin.js Outdated Show resolved Hide resolved
@LokiLuciferase
Copy link
Contributor Author

Thanks for the review, I made the requested changes.

@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Mar 14, 2023
thornbill
thornbill requested changes Mar 14, 2023
View reviewed changes
src/plugins/bookPlayer/plugin.js Outdated Show resolved Hide resolved
@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Mar 14, 2023
@sonarcloud
Copy link

sonarcloud bot commented Mar 24, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@thornbill thornbill merged commit 273dfa0 into jellyfin:master Mar 28, 2023
@LokiLuciferase LokiLuciferase deleted the feature/bookplayer branch March 28, 2023 19:23
@kamanwu
Copy link

kamanwu commented Jan 1, 2024

I use the latest jellyfin 10.8.13. When I read ePub books, I only can see 2 buttons: Go to contents | full screen.

I cannot change font size. Do I miss something? Thanks.

@thornbill
Copy link
Member

New features do not get released in bugfix/patch releases.

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

@thornbill thornbill thornbill approved these changes

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

Successfully merging this pull request may close these issues.

EPUB support is broken with current master branch due to epubjs 0.4.2 incompatibility
4 participants
@LokiLuciferase @kamanwu @thornbill @jellyfin-bot