Implement book finished screen with total words read after each book#672
Implement book finished screen with total words read after each book#672VeryUsual wants to merge 1 commit intoLuteOrg:developfrom
Conversation
VeryUsual
commented
Apr 19, 2026
VeryUsual
commented
VeryUsual
force-pushed
the
book-finished-screen
branch
from
3c1c4fe to
db963b4
Compare
axel584
left a comment
axel584
left a comment
There was a problem hiding this comment.
It works well on my machine, and I like the visual appearance of the result.
I find the code very clean. Using the service to calculate the word count is very effective.
Great work!
There was a problem hiding this comment.
@VeryUsual , I'd like the message to be shown in a div within the reading page, for the reasons given above. If you have time to make those changes, great, and if not I can use the work you've done here to get it in. Does that make sense?
Cheers and thanks very much!
edit: PS - I've contributed to open source projects before, and sometimes getting pushback like this from the owners feels a bit thankless. I hope that's not the case here, I appreciate you taking the time to push the project forwards constructively.
| gap: 10px; | ||
| justify-content: center; | ||
| } | ||
|
|
There was a problem hiding this comment.
Trying this out locally.
- the .btn and btn-xxxx styles shouldn't be added. Those classes are added to buttons elsewhere, and these colors don't work in general. Eg for the language screen, see the screenshot below.
| <p>Great job!</p> | ||
| <p class="words-read">You've read <strong>${total_words}</strong> words.</p> | ||
| <div class="book-finished-actions"> | ||
| <a href="/" class="btn btn-primary">Home</a> |
There was a problem hiding this comment.
Removing the .btn style will make this "home" link to just be a link, not a button :-) Currently, the last page of the book an "archive" link, which I kind of like having. I think that just having a "Close" button is sufficient -- or, perhaps best would be to just have the message get put into a div in the page reading page itself, without the popup overlay. See the notes in the popup overlay css above.
There was a problem hiding this comment.
After thinking through the styling implications, I do feel that a method like showFinishedMessage would be best.
| @@ -998,6 +998,99 @@ div.code pre { | |||
| background-image: url("/static/icn/font-decrease.svg"); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
This is good on its own, but these prescribed styles won't work well for some themes. Some users have specific displays (e.g grayscale) that require certain colors, so setting colors here means that all themes need to be touched as well, which is a drag.
Rather than do a popup, I think that the cleanest and least intrusive method would be to show the "congratulations" message in the simple system font, just as a div within the reading page. Fewer popups = fewer style issues, and less styling code etc overall.
3 participants
