-
-
Notifications
You must be signed in to change notification settings - Fork 89
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
Feature/reading status improvements #977
base: master
Are you sure you want to change the base?
Feature/reading status improvements #977
Conversation
This commit introduces a new feature that allows users to hide books they have completed reading from the library view. Key changes: - Add 'Hide read books' menu item in headerbar - Implement book read status detection using progress/duration comparison - Update library view model to filter read books when enabled Signed-off-by: Adrien Kara <[email protected]>
Add status icons to book cards that show reading progress: - Add status icon overlay in book card UI - Show check icon for completed and headphone icon for books in progress - Style icons with appropriate colors (success/accent) Signed-off-by: Adrien Kara <[email protected]>
- Add dynamic menu creation to show/hide read/unread options based on book state - Add mark_as_unread method to reset book and chapter positions - Add unread action to main view controller This improves the UX by only showing relevant menu options based on the current state of the book. Signed-off-by: Adrien Kara <[email protected]>
Signed-off-by: Adrien Kara <[email protected]>
Signed-off-by: Adrien Kara <[email protected]>
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.
Hello, and thanks for these contributions! I've added some review comments above.
It looks like you didn't revert the flatpak manifest file correctly, as there's still diff compared to the main branch. Could you update it from the master branch (git checkout origin/master -- com.github.geigi.cozy
), and commit that?
|
||
item { | ||
action: 'app.hide_read'; | ||
label: _("_Hide read books"); |
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.
H
as a mnemonic clashes with the other action's H
from above. I'd consider using the r
here.
[overlay] | ||
Revealer played_revealer { | ||
transition-type: crossfade; | ||
valign: start; | ||
halign: start; | ||
|
||
Image status_icon { | ||
margin-start: 6; | ||
margin-top: 6; | ||
pixel-size: 20; | ||
tooltip-text: _("Book status"); | ||
|
||
styles [ | ||
"circular", | ||
"opaque", | ||
"status-icon", | ||
] | ||
} | ||
} | ||
|
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.
TBH, I don't quite like this icon thing, it makes the overall UI a bit cluttered. Maybe it could be shown only when the card is hovered, like we do with the menu and play button.
Earlier we used the play button's progress ring to mark books as read, by turning it green. Currently that wouldn't work, because now we use the accent color for the play button, which can be green, but I'm sure the read status could be integrated into it somehow.
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.
Hi @rdbende,
Thank you for your feedback! I understand your concerns about the UI potentially appearing cluttered with the current icon implementation. However, I'd like to share my perspective as a user.
When navigating through the recent books interface, it can be challenging (to me) to remember which books I've completed, without having to hover over each one individually. This can become quite tedious. Having a visual indicator at a glance significantly enhances my user experience.
For me, it's important to consider accessibility for users with visual impairments, who are a significant target audience for this type of software. Don't you think ?
This implementation provides an icon that is beneficial for individuals with color blindness or monochromacy, and offers textual information for screen readers (although I haven't been able to test this personally). Relying only on color to convey status, such as using the play button's color, may not be effective for everyone and the symbolism of color can be misinterpreted. And we can't rename the play button in "read" without compromising its clarity. Have you a better proposal ?
The colors used are aligned with the "success" and "accent" themes of the GNOME project. The appearance may differ slightly on my example due to the Dracula theme.
🤔
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.
So I went and looked at how audiobook distributors do things.
- Audible is ugly, but still displays the status in text.
- Apple uses a little triangle in the top right corner to display the status
- Google Play Book only seems to display a blue icon in the bottom right-hand corner when a book is finished.
For video streaming, I've noticed that Netflix, Prime and Disney+ use a progress bar and Apple an icon.
In all cases, this information is always visible without hover or any user action, which is really handy.
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.
When you add an icon, you also need to add an entry for it in the gresource file, otherwise it won't get bundled.
Reading Status Improvements
Hi there :)
Overview
This MR introduces a set of features to enhance the book reading status management in Cozy. The changes focus on improving the user experience by providing better visual feedback and control over the library organization.
Fix: #351
Key Features
Technical Implementation
The implementation spans across :
Testing
Screenshots
Apologies
Please note that the last commit 6ddbf8c is a restoration of the original
com.github.geigi.cozy.json
. An automatic formatter from Builder had modified the file and I inadvertently committed changes, this commit restores it.