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

Remove podcast shortcuts on home #36

Closed
wants to merge 2 commits into from
Closed

Conversation

hazre
Copy link

@hazre hazre commented Sep 26, 2022

if this can be better implemented, please modify it.

@hazre hazre marked this pull request as ready for review September 26, 2022 11:56
@theRealPadster
Copy link
Owner

theRealPadster commented Sep 26, 2022

Hey, first of all I wanted to say thanks for this. I took a look and I can't add a wait for .view-homeShortcutsGrid-grid because the app may not start on the home screen, and the code would get stuck there. e.g. if running spicetify enable-devtools, or if you change the homepage, etc. Second, I don't even have .view-homeShortcutsGrid-grid on my home screen. Which version of Spotify are you using? I'm on 1.1.94.872.

If my Spotify is outdated, I may need to do that and take a look at the classes and integrate your changes in.

@hazre
Copy link
Author

hazre commented Sep 26, 2022

Hi, I've tested it locally and it seems to work. I'm using the latest version (1.1.94.872.g7a9200fe).

video of me, building and applying the pr changes:

Spotify_2022-09-26_16-01-53.mp4

app may not start on the home screen

I think then it could be added to the listenThenApply() function.

@theRealPadster
Copy link
Owner

theRealPadster commented Sep 29, 2022

Ohh, it looks like you've got the new UI in your Spotify. It's still in A/B testing and hasn't rolled out to me yet. I'm guessing it's got different structure/classes.

I may need to wait until I've got the new UI before I can properly mess around and figure out the best way to handle it.
(e.g. detect .view-homeShortcutsGrid-grid just when on the homepage, or if it's present on all pages, or if all pages load in content async now and I need to add each of their containers, etc. )

@theRealPadster theRealPadster self-assigned this Sep 29, 2022
@theRealPadster theRealPadster added bug Something isn't working blocked Progress is held up by something else labels Oct 3, 2022
@theRealPadster
Copy link
Owner

theRealPadster commented Oct 27, 2022

Hey, I finally got around to fixing things for the new UI. I thought there was an issue about it being broken, but couldn't find it, so fixed the issue I had my own way. Could you give the latest release (2.5.2) a try, and let me know if your issue is still a problem? Again, sorry, I completely forgot it was a PR and not an issue. Happy to take another look if it's still broken for you.

I also checked and I still don't have .view-homeShortcutsGrid-grid in my home page DOM.

@hazre
Copy link
Author

hazre commented Oct 30, 2022

They still appear 🫤

Spotify_2022-10-30_15-32-04

image

Comment on lines +74 to +78

// remove podcast shortcuts on home
document.querySelectorAll('.view-homeShortcutsGrid-name a[href^="/episode"]').forEach((selected) => {
selected.parentNode.parentNode.parentNode.parentNode.parentNode.classList.add('podcast-item');
});
Copy link
Owner

Choose a reason for hiding this comment

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

So I have the .view-homeShortcutsGrid-grid elements in my DOM now. I don't have a podcast item, but your code looks sound. I just cleaned it up a little. Was going to push up to this PR but my VS Code is being weird and creating a new branch on the main repo when I try and push, so I'll just put this here.

Suggested change
// remove podcast shortcuts on home
document.querySelectorAll('.view-homeShortcutsGrid-name a[href^="/episode"]').forEach((selected) => {
selected.parentNode.parentNode.parentNode.parentNode.parentNode.classList.add('podcast-item');
});
// Remove podcast shortcuts on home page
const podcastShortcutsOnHomepage = document.querySelectorAll('.view-homeShortcutsGrid-name a[href^="/episode"]');
podcastShortcutsOnHomepage.forEach((itemLink) => {
const item = itemLink.closest('.view-homeShortcutsGrid-shortcut');
if (item) item.classList.add('podcast-item');
});


while (!Player || !Menu || !Platform || !mainElem) {
while (!Player || !Menu || !Platform || !mainElem || !shortcutsElem) {
Copy link
Owner

Choose a reason for hiding this comment

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

I also don't think we can put this here, since the app may not start on the homepage (and their Spotify may not even have it).

It might be a better idea to add a custom element to check for on the homepage down in the listenThenApply function, like how it does for the /search page.

@theRealPadster
Copy link
Owner

@hazre Sorry for the delay here. I took another stab at this and think it should be fixed in the latest version. Could you take a look and see if it works for you? This is the fix PR for reference

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Progress is held up by something else bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants