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

Fix: sh.reddit.com search and compact view not loading #1452

Merged
merged 31 commits into from
Nov 6, 2024

Conversation

LiliaDoe
Copy link
Contributor

@LiliaDoe LiliaDoe commented Nov 4, 2024

Made sh.reddit search and compact view media load.

  • In searches, selecting only the thumbnails allowed it to load, while in normal compact view, selecting the whole faceplate allowed it to load. Not sure why. I want to streamline it, but for now, things work as expected.
  • Might allow searches to load media in old.reddit as well in the future

@LiliaDoe
Copy link
Contributor Author

LiliaDoe commented Nov 4, 2024

I'll work on reducing the duplication later

@extesy
Copy link
Owner

extesy commented Nov 5, 2024

When you are going to test this, please make sure that old.reddit.com still works fine. Old reddit is the best reddit.

@LiliaDoe
Copy link
Contributor Author

LiliaDoe commented Nov 5, 2024

Do I need to reduce the multiple gallery selectors? I can work on it, but I see that was there from previous commits. I'm a little afraid to change any of it in case some part of old.reddit with a gallery stops working, to be honest.

@LiliaDoe
Copy link
Contributor Author

LiliaDoe commented Nov 5, 2024

I've tried reducing the duplicate lines, and i think I got it as low as I can without changing previous code too much. There were more errors with my first committed changes than I expected, but after looking over it for a while, everything is working. The places I tested on old.reddit.com and sh.reddit.com are: galleries, images, and videos, in subreddits and home feed. In a future pull request, I can make media in old.reddit.com searches show when hovering their thumbnail or header too, if it'd be helpful

@extesy
Copy link
Owner

extesy commented Nov 6, 2024

In a future pull request, I can make media in old.reddit.com searches show when hovering their thumbnail or header too, if it'd be helpful

Up to you. I don't remember users ever complaining about a lack of zoom on the search results. You could fix it if you'd like, but it is a low priority.

There were more errors with my first committed changes than I expected, but after looking over it for a while, everything is working.

When you think this PR is ready for testing, please mark it non-draft, then I'll pull the branch to double check it myself. But ultimately, we'll have see from users bug reports if something was accidentally broken :)

Copy link

sonarcloud bot commented Nov 6, 2024

@LiliaDoe
Copy link
Contributor Author

LiliaDoe commented Nov 6, 2024

I looked over the whole file again, and made sure the old code was how it was when I found it, with updated variable scopes. My only concern is new reddit tends to run slower than old reddit, but I think that's just the nature of the websites rather than a problem with HZ+

@LiliaDoe LiliaDoe marked this pull request as ready for review November 6, 2024 18:01
@extesy
Copy link
Owner

extesy commented Nov 6, 2024

Seems to work for me. Thank you!

@extesy extesy merged commit 7ae5e25 into extesy:master Nov 6, 2024
2 checks passed
@LiliaDoe LiliaDoe deleted the Fix-sh.reddit.com-search-not-zooming branch November 6, 2024 22:15
@LiliaDoe
Copy link
Contributor Author

LiliaDoe commented Nov 6, 2024

Also, this fixes #1390 and possibly #458

@LiliaDoe
Copy link
Contributor Author

LiliaDoe commented Nov 6, 2024

If users report issues with sh.reddit.com searches not zooming when mouse hasn't moved, one could resolve it by changing:

    $('faceplate-tracker[data-faceplate-tracking-context*="post_thumbnail"]').each(function () {

to

    $('faceplate-tracker[data-faceplate-tracking-context*="post_thumbnail"]').one('mouseover', function () {

Using .each() just makes images load faster

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