Skip to content

Conversation

@Nokse22
Copy link
Owner

@Nokse22 Nokse22 commented Aug 4, 2025

This MR includes various things:

  • Cache to make the app more responsive
  • Simplified and unified pages
  • More doc strings
  • Type hints
  • Various cleanups (formatting, removing unused functions ...)
  • Improved and cleaned up some widgets API

Nokse22 added 2 commits August 4, 2025 21:15
Gtk is not thread safe, so this way all gtk calls should be made in the main thread

Added some helper functions to Page to make populating pages easier.
@Nokse22 Nokse22 changed the title Shortcuts widget Shortcuts widget and page loading rework Aug 4, 2025
@Nokse22 Nokse22 changed the title Shortcuts widget and page loading rework Page loading rework and more Aug 8, 2025
@Nokse22 Nokse22 marked this pull request as ready for review August 8, 2025 09:11
@Nokse22 Nokse22 requested a review from nilathedragon August 8, 2025 09:18
@drafolin
Copy link
Collaborator

drafolin commented Aug 8, 2025

bro has been cooking :0

I say we do a major release when this is merged (so 1.0.0)

@Nokse22
Copy link
Owner Author

Nokse22 commented Aug 8, 2025

bro has been cooking :0

hahahahah, thanks

I say we do a major release when this is merged (so 1.0.0)

Was thinking this too

@Nokse22 Nokse22 requested a review from drafolin August 8, 2025 16:00
Copy link
Collaborator

@drafolin drafolin left a comment

Choose a reason for hiding this comment

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

LGTM; will check locally

@Nokse22
Copy link
Owner Author

Nokse22 commented Aug 8, 2025

@youwen5 I figured out the issue with your first screenshot, but since my account doesn't have anything Featured I can't test it, can you tell me if it's fixed?

@nilathedragon
Copy link
Collaborator

Tested it on NixOS /w my development flake. Looking good, and speedy too!

As for the "Featured" thing, it renders a bit different but the buttons do nothing.

image

@nilathedragon
Copy link
Collaborator

Hitting the < > buttons on these gives me an error

TypeError: Page.carousel_go_prev() takes 3 positional arguments but 4 were given
TypeError: Page.carousel_go_prev() takes 3 positional arguments but 4 were given
TypeError: Page.carousel_go_next() takes 3 positional arguments but 4 were given
TypeError: Page.carousel_go_prev() takes 3 positional arguments but 4 were given
TypeError: Page.carousel_go_next() takes 3 positional arguments but 4 were given
TypeError: Page.carousel_go_prev() takes 3 positional arguments but 4 were given
TypeError: Page.carousel_go_next() takes 3 positional arguments but 4 were given
TypeError: Page.carousel_go_next() takes 3 positional arguments but 4 were given
TypeError: Page.carousel_go_prev() takes 3 positional arguments but 4 were given
TypeError: Page.carousel_go_prev() takes 3 positional arguments but 4 were given
TypeError: Page.carousel_go_next() takes 3 positional arguments but 4 were given
image

@Plamper
Copy link
Collaborator

Plamper commented Aug 8, 2025

The featured stuff also has some issues on my end. I can't click on the items and only 4 are shown even if there is space for six (I think this is from tidal. Maybe a bit wider icons like the official version). If I click on the small arrows, two of them disappear. Also, I think they should have images.

image image image

@Plamper

This comment was marked as resolved.

@Plamper

This comment was marked as resolved.

@Nokse22
Copy link
Owner Author

Nokse22 commented Aug 9, 2025

I hope the issue with featured items it's now fixed. If someone can confirm it I can merge this.

@Nokse22 Nokse22 requested review from Plamper and drafolin August 9, 2025 13:39
Copy link
Collaborator

@drafolin drafolin left a comment

Choose a reason for hiding this comment

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

just two changes that could improve code readability and DX.
I am currently in the process of reinstalling my system so i can't check if it works right now 😅

@nilathedragon
Copy link
Collaborator

I hope the issue with featured items it's now fixed. If someone can confirm it I can merge this.

They render correctly for me now and they also go to the right thing. The Link carousels are also working now.

image

@Nokse22 Nokse22 requested a review from drafolin August 9, 2025 14:12
Copy link
Collaborator

@drafolin drafolin left a comment

Choose a reason for hiding this comment

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

Just tested in an arch gnome box, and it definitely works

@Nokse22 Nokse22 merged commit ccc9e35 into master Aug 9, 2025
3 checks passed
@Nokse22 Nokse22 deleted the shortcuts-widget branch August 9, 2025 15:07
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.

6 participants