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.

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

Redesign book detail view #948

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from
Draft

Conversation

Haaruun-I
Copy link
Contributor

@Haaruun-I Haaruun-I commented Jul 31, 2024

Should close #945 and #943

This adds a tooltip showing the file name of the chapter when you hover over it, and a fallback image. Also changes out the weird progress ring and labels with a progress bar.

Screenshot from 2024-07-31 09-29-15

Also gives two lines to the title text, instead of just one. And moves the playbutton to the right hand side.

Screenshot from 2024-07-31 09-25-08

Here is it without those two changes, I think the 2 lines look better, but not sure on what side the play button should be on

Screenshot from 2024-07-31 09-23-50

Also adds to the options menu, giving it the same options as in the card + the now renamed download button (greyed out because the option isnt availible, would just be hidden normally)

image

Should the preview card also have the download option? I think it would make sense if the two menus had all the same options

Going to keep this as a draft untill I get feedback on the design

@Haaruun-I Haaruun-I marked this pull request as draft July 31, 2024 06:38
@Haaruun-I Haaruun-I changed the title Book detail view Redesign book detail view Jul 31, 2024
accidentally commited this sometime, whoops
@@ -258,7 +258,7 @@ menu book_menu_model {
section {
item {
action: 'book_overview.download';
label: _("_Available Offline");
label: _("Download Book");
Copy link
Collaborator

@rdbende rdbende Aug 12, 2024

Choose a reason for hiding this comment

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

This is a two-state action, that's why I went with "Available Offline".

Copy link
Collaborator

@rdbende rdbende left a comment

Choose a reason for hiding this comment

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

Thank you for working on this!
I've left a few comments, but the changes are great overall. The horizontal progress bar indeed works better, however there's a reason I added a circular progressbar not so long ago. On mobile view, the long progressbar doesn't really make sense, and the labels would be cut off anyways, so I'd keep the circular one for when the screen is too narrow (ie. on the smallest breakpoint size)

data/ui/book_detail.blp Outdated Show resolved Hide resolved
data/ui/book_detail.blp Outdated Show resolved Hide resolved
data/ui/book_detail.blp Outdated Show resolved Hide resolved
cozy/ui/book_detail_view.py Outdated Show resolved Hide resolved
data/ui/book_detail.blp Show resolved Hide resolved
self.album_art.set_paintable(paintable)
self.album_art.set_overflow(True)
self.album_art_container.set_visible_child(self.album_art)
else:
self.fallback_icon.set_from_icon_name("book-open-variant-symbolic")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be set from the Blueprint file

@rdbende
Copy link
Collaborator

rdbende commented Aug 12, 2024

@DjLizama please don't approve pull requests without actually reviewing it. Thanks!

@Haaruun-I
Copy link
Contributor Author

Thank you for the review! I think the progress bar works fine on narrower screens, although the rest of the top bar could use a little work.

image

@rdbende
Copy link
Collaborator

rdbende commented Aug 14, 2024

image

Hmm. Maybe it should display times in the 00:00:41, 10:42:00 format then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Poor use of space truncates chapter titles
3 participants