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

Homepage revamp #520

Merged
merged 32 commits into from
Sep 23, 2024
Merged

Homepage revamp #520

merged 32 commits into from
Sep 23, 2024

Conversation

SO9010
Copy link
Contributor

@SO9010 SO9010 commented Sep 5, 2024

This aims to sort #519 ! Please message there if there's anything you would want to see implemented! here is the current rough rough rough state of this PR!

Screencast.from.2024-09-05.21-00-56.mp4

@jacksongoode
Copy link
Collaborator

jacksongoode commented Sep 5, 2024

The only other project that does retrieve these made for you sections is Spotube. The hidden Spotify endpoints they discovered are here. It might be helpful in your redesign. We should be adding these to home regardless but their API is a little tricky for these I think - they need to be destructured in a particular way.

https://github.com/KRTirtho/spotube/blob/9b024120601c0d381edeab4460cb22f87149d0f8/lib/services/custom_spotify_endpoints/spotify_endpoints.dart

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 5, 2024

That has made me very excited, I also spotted a "what are your friends listening to" section! I'll happily work on working out the apis. Do you think the current drafts layout is effective, we could possibly keep the "your top artists and tracks" section at the bottom as I personally really like being able to use it.

@jacksongoode
Copy link
Collaborator

Having a look at this now, and will give some feedback!

@jacksongoode
Copy link
Collaborator

Overall I like it! Some thoughts:

  • The thumbnails are a bit too large, I don't think its in keeping with the existing UI of the application
  • Some of the descriptions are garbled. I don't think this is an issue with this PR but rather how we sanitize the descriptions as a whole. HTML tags are preserved.
  • I think we need a little bit more padding between the vertical sections so it feels a little but more like the division in the search (album, playlist, tracks)
image

.gitignore Outdated Show resolved Hide resolved
@jacksongoode
Copy link
Collaborator

I made a small commit that does the sanitization of tags and unifies the playlist and artist thumbnail heights, I made them a bit smaller to be more cohesive with the UI. I think we should probably sanitize the HTML at some earlier step getting the playlist and the widget should be drawing from the existing way we pull playlist information - I feel there is quite a bit of duplicate code as it stands.

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 12, 2024

The changes you made look lovely! Thank you!!! I agree I will try and reduce the code once the functionality is there :)

@jacksongoode
Copy link
Collaborator

Oh, I also wanted to mention, because I saw it in some comment in the code, that all users should be able to scroll horizontally by holding Shift and scrolling. At least that's how it works on Mac and Windows. So if you have your cursor over the horizontal scroll section, you should be able to press Shift and scroll and it'll scroll through.

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 13, 2024

Ah, perfect. Thank you; that makes it a lot easier for me! I've now added almost all of the home sections to the playlists. Should I use the UI for the search section to have horizontal sections?

@jacksongoode
Copy link
Collaborator

I think that should probably be a separate PR so we can just change one view at a time. I think there's some expectation with a search to see a list of items.

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 13, 2024

Ok perfect thanks!

…and episodes for you! Also remove changes to uneeded places
@jacksongoode
Copy link
Collaborator

jacksongoode commented Sep 23, 2024

I made just a few cleanup changes.

  • I reduced the height of the standard widget from 8.5 to 7 so that it's 3 lines of height.
  • I also reduced the number of characters allowed so that we can be certain that we stick to three lines and don't overflow.
  • I added a small padding to the top and bottom of the title widgets to just give a little bit more vertical separation between the sections.
  • Changed the home icon to the filled one and make it smaller to fit the path size

@jacksongoode
Copy link
Collaborator

If those changes look good to you we can merge!

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 23, 2024

Thank you for these changes, i think that we need to give a little extra height to allow some extra space at the bottom, as that gets cut off by the scroll bar!

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 23, 2024

Cool, I changed it to be 8.0. If you are happy with it, go ahead and merge! Thank you. By the way, I enjoyed working on this!

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 23, 2024

Also, I was wondering if we should redo the saved podcasts section to include recommended episodes and shows. It should be pretty easy to do.

@jacksongoode jacksongoode merged commit 4dfa43d into jpochyla:master Sep 23, 2024
8 checks passed
@SO9010 SO9010 deleted the Improve-views branch September 23, 2024 21:56
@SO9010 SO9010 restored the Improve-views branch September 23, 2024 23:05
@SO9010
Copy link
Contributor Author

SO9010 commented Sep 23, 2024

@jacksongoode Weirdly, I think the jump-back-in section has gone off Spotify now... How weird! I'll check back in the morning, but if it still isn't there (I mean on the Spotify website), I'll submit a quick patch. How annoying :/

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 23, 2024

Wait it is still a thing! Why isn't it loading the data properly anymore I haven't changed anything!? Could you check this on your machine @jacksongoode

@jacksongoode
Copy link
Collaborator

Oh no I still see it on my end, can you debug the call?

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 23, 2024

Oh its just my laptop giving up! I cleared the cache and its all fine now :) sorry for the alarm bells!

@SO9010 SO9010 deleted the Improve-views branch September 23, 2024 23:49
@jacksongoode
Copy link
Collaborator

What cache did you clear? I'm actually noticing the same issue @SO9010

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 24, 2024

@jacksongoode I deleted the whole psst folder for all the cache and then restarted my laptop, but maybe if it's not so stable we should remove it from the home page?

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 24, 2024

So very weird that it only just became a problem and only for that section. I've just checked my machines again and it is fine on both windows and Linux ATM

@jacksongoode
Copy link
Collaborator

Hmm I did remove the cache but still get the error...

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 25, 2024

@jacksongoode is this still an error? Its still ok on my ends!

@arch-btw
Copy link
Contributor

I'm also getting an error, it's looking for missing fields in the json file apparently. Screenshot:

psst

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 25, 2024

This is so odd that this has only just turned up as an issue! I submitted a PR to fix the track adding. But maybe we should revert the merge to main? I can start reworking it.

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 25, 2024

I'm thinking this has got to do with the API call request to get the users timezone and location. We should get the user to set this up at the start. Maybe there's a crate which can be used.

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 25, 2024

@arch-btw are you using a VPN I'm thinking that could be the issue as it currently uses the computers IP address

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 25, 2024

Okay, I don't think that the issue is coming from the IP address, as I just tried a VPN, and also, your uniquely yours wouldn't show up if it were. I also just had an idea: We could have it so that instead of showing the error, it only loads the items it can get, so we just display an empty widget. @jacksongoode, what do you think?

@SO9010 SO9010 restored the Improve-views branch September 25, 2024 16:27
@arch-btw
Copy link
Contributor

@arch-btw are you using a VPN I'm thinking that could be the issue as it currently uses the computers IP address

Yes, well sort of, I'm using the socks5 feature like this:

SOCKS_PROXY=socks5://<ip address>:<port> psst-gui

It was introduced here: #20

Sorry, I wish I could help more.

@SO9010
Copy link
Contributor Author

SO9010 commented Sep 25, 2024

@arch-btw don't worry, just out of curiosity how does the home layout look for you now after the most recent merge

@arch-btw
Copy link
Contributor

@SO9010

Oops, I didn't see your message! Here is what it currently looks like.
By the way, these upcoming features were a really good idea, I look forward to trying them when they are ready.
Let me know if you need anything else :)

psst2

@jacksongoode
Copy link
Collaborator

@SO9010 We should probably be also have the title appear contingent on the panel's network request actually loading.

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.

3 participants