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

Minor improvements and fixes to library #2816

Merged
merged 6 commits into from
Apr 15, 2023

Conversation

RozeFound
Copy link
Contributor

@RozeFound RozeFound commented Mar 23, 2023

Description

Fixed "no cover" text not being shown
Centered items in LibraryView, I think it's look better, but if you don't agree, I can revert this

Fixes #2458

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

I dont know how to test it tbh...

@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2023

Pylint result on modfied files:
************* Module bottles.frontend.views.library
bottles/frontend/views/library.py:29:0: W1405: Quote delimiter ' is inconsistent with the rest of the file (inconsistent-quotes)
bottles/frontend/views/library.py:31:0: W1405: Quote delimiter ' is inconsistent with the rest of the file (inconsistent-quotes)
bottles/frontend/views/library.py:22:0: E0611: No name 'Adw' in module 'gi.repository' (no-name-in-module)
bottles/frontend/views/library.py:60:12: C0103: Variable name "u" doesn't conform to snake_case naming style (invalid-name)
bottles/frontend/views/library.py:60:15: C0103: Variable name "e" doesn't conform to snake_case naming style (invalid-name)
bottles/frontend/views/library.py:68:0: W0613: Unused argument 'args' (unused-argument)
bottles/frontend/views/library.py:73:0: W0613: Unused argument 'args' (unused-argument)
bottles/frontend/views/library.py:90:22: W0613: Unused argument 'widget' (unused-argument)
bottles/frontend/views/library.py:19:0: W0611: Unused import re (unused-import)

@RozeFound
Copy link
Contributor Author

RozeFound commented Mar 23, 2023

Sorry for PR without opening an issue again...

Also I want to clarify why am I contributing to Bottles all of a sudden.
I currently working on my own GTK4/Adwaita application, and I look at Bottles code as an example a lot, and sometimes spotting some bugs like this
I don't use any of assets of course, and don't straight copy-paste any code, so I hope you're fine with that

@TheEvilSkeleton
Copy link
Contributor

That's fine. To be fair, your MRs have been really useful so far and I really like them. Feel free to join our Discord server if you want some help with development :)

TheEvilSkeleton

This comment was marked as resolved.

@RozeFound
Copy link
Contributor Author

Hmm, I'll look into it

@RozeFound RozeFound marked this pull request as draft March 24, 2023 19:43
@RozeFound
Copy link
Contributor Author

How does it look now?

@RozeFound RozeFound marked this pull request as ready for review March 24, 2023 21:02
@TheEvilSkeleton
Copy link
Contributor

TheEvilSkeleton commented Mar 24, 2023

Honestly, I really like how "No Thumbnail" looks, but we'll have to discuss its looks in #2458

The rest looks great 👌

TheEvilSkeleton

This comment was marked as resolved.

@RozeFound
Copy link
Contributor Author

RozeFound commented Mar 24, 2023

Honestly, I really like how "No Thumbnail" looks, but we'll have to discuss its looks in #2458

Well, that was not me who added that, it was in code just disabled

@TheEvilSkeleton
Copy link
Contributor

Oh, 😅

Copy link
Contributor

@TheEvilSkeleton TheEvilSkeleton left a comment

Choose a reason for hiding this comment

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

Hmm... it looks like it gets "stuck" when you remove an entry, but centers when you restart the app:

Screencast.from.2023-03-24.21-51-06.webm

Can you make it centered all the time?

@RozeFound
Copy link
Contributor Author

That's weird...

@TheEvilSkeleton
Copy link
Contributor

Yeah :/

@RozeFound
Copy link
Contributor Author

oh. self.update() not called on entry removal, that's why

@RozeFound
Copy link
Contributor Author

Sorry to make you test every little change... But this time I hope it'll work

@TheEvilSkeleton
Copy link
Contributor

Don't worry about it. I do QA a lot, so I'm used to it :)

@TheEvilSkeleton
Copy link
Contributor

I just tested it, and it looks good to me. I'll let it sit for a bit and wait until we figure out #2458... unless it takes a really long time

@TheEvilSkeleton TheEvilSkeleton added this to the 52.0 milestone Apr 15, 2023
Copy link
Contributor

@TheEvilSkeleton TheEvilSkeleton left a comment

Choose a reason for hiding this comment

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

Looks like there won't be any response anytime soon. I'll just merge it. Thank you once again!

@TheEvilSkeleton TheEvilSkeleton merged commit 8bb19bb into bottlesdevs:main Apr 15, 2023
@TheEvilSkeleton
Copy link
Contributor

TheEvilSkeleton commented Apr 15, 2023

I dont know how to test it tbh...

@RozeFound I just noticed this in the original comment. If you want to test next time, click on the Details button in the "Build Flatpak" field at the bottom of the MR. It should look like this:

image

You won't see this button in this MR, as I just merged it. You can open another MR and try it there. Afterwards, press "Summary", and then click "bottles-x86_64" under "Artifacts". It should download a zip archive. Then, unzip it, and finally run flatpak install bottles.flatpak to install it. I highly recommend backing up ~/.var/app/com.usebottles.bottles before testing experimental builds.

When you're done testing, you can run flatpak remove --no-related com.usebottles.bottles//master to uninstall the experimental build.

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.

library: Design issues
3 participants