Skip to content

feat: Improve search ui, add offsets to dab search#1

Merged
replydev merged 3 commits intoreplydev:feat/dab-music-integrationfrom
MalTeeez:feat/dab-music-integration
May 5, 2025
Merged

feat: Improve search ui, add offsets to dab search#1
replydev merged 3 commits intoreplydev:feat/dab-music-integrationfrom
MalTeeez:feat/dab-music-integration

Conversation

@MalTeeez
Copy link

@MalTeeez MalTeeez commented May 3, 2025

This PR implements the UI changes as presented in the upstream PR freeman-jiang#18.
It also adds the search with offset functionality needed for the pagination controls along the necessary backend features and some refactoring of the download / search interface for future usage.

@MalTeeez
Copy link
Author

MalTeeez commented May 3, 2025

Oh, and i also added a regenerated bun.lock to fix freeman-jiang#21

Copy link
Owner

@replydev replydev left a comment

Choose a reason for hiding this comment

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

Hello @MalTeeez, thanks for your changes.

I think we are missing the actual part that handles the offset parameter, in DOWNLOAD_SERVICE (maybe you forgot to commit?)

Then, I sent just a simple request change, but as far as I saw from your screenshot, this is pretty good!

Finally, I guess we need a proper prettier / biome config to handle formatting, because most of the changes are indents and a bit difficult to review.

I will check this hands on, today.

@MalTeeez
Copy link
Author

MalTeeez commented May 4, 2025

Hello @MalTeeez, thanks for your changes.

I think we are missing the actual part that handles the offset parameter, in DOWNLOAD_SERVICE (maybe you forgot to commit?)

Yep, seems they were missing somehow - working across 3 repositories & multiple branches is getting a little tricky.

I also added tooltips for list elements, in case some titles / albums get too long.

Finally, I guess we need a proper prettier / biome config to handle formatting, because most of the changes are indents and a bit difficult to review.

Totally agree, but probably something for upstream to start in a new PR.

@MalTeeez
Copy link
Author

MalTeeez commented May 5, 2025

The whole component does still need to be improved for mobile, it turns unusable as soon as the control components leave the screen..

@replydev
Copy link
Owner

replydev commented May 5, 2025

The whole component does still need to be improved for mobile, it turns unusable as soon as the control components leave the screen..

For mobile we could use the credenza component, you can find it on Github.

It's basically a wrapper that chooses which component to use given the screen size.

Ofc it's not needed here, but as a further enhancement later on 👍.

@replydev replydev merged commit 460cb54 into replydev:feat/dab-music-integration May 5, 2025
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.

2 participants