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: images showing when mouse is off image link #1515

Merged
merged 2 commits into from
Dec 29, 2024

Conversation

LiliaDoe
Copy link
Contributor

@LiliaDoe LiliaDoe marked this pull request as ready for review December 29, 2024 08:00
@LiliaDoe LiliaDoe changed the title Fix: images loading when mouse is off image link Fix: images showing when mouse is off image link Dec 29, 2024
@extesy
Copy link
Owner

extesy commented Dec 29, 2024

This PR fixed the problem, so I'm merging it.

@extesy extesy merged commit 4cb6b61 into extesy:master Dec 29, 2024
3 checks passed
@LiliaDoe
Copy link
Contributor Author

Glad to hear!

@LiliaDoe LiliaDoe deleted the Bug-fix branch December 29, 2024 22:21
@extesy
Copy link
Owner

extesy commented Dec 30, 2024

@LiliaDoe I spoke too soon, I'm still seeing this problem. Repro steps (can repro on reddit as well but this one is easier):

  1. Open https://forums.crateentertainment.com/t/announcing-fangs-of-asterkarn/129879
  2. Set "delay before displaying a picture" to some reasonably high value.
  3. Briefly move the cursor into the picture and immediately move it far away.

Observed: the zoomed mage is still displayed even though the mouse is no longer hovering over it.
Desired: only display the image if the mouse is hovering over it.

@LiliaDoe LiliaDoe restored the Bug-fix branch December 30, 2024 23:44
@LiliaDoe
Copy link
Contributor Author

LiliaDoe commented Dec 31, 2024

Ok, I can reproduce it consistently now.

@LiliaDoe
Copy link
Contributor Author

LiliaDoe commented Dec 31, 2024

I believe I found the source of the issue:

hoverzoom/js/hoverzoom.js

Lines 1074 to 1086 in 4cb6b61

if (audioSrc) {
chrome.runtime.sendMessage({action:'isImageBanned', url:audioSrc}, function (result) {
if (!result) {
loadFullSizeImageTimeout = setTimeout(loadFullSizeImage, delay);
}
});
} else if (src) {
chrome.runtime.sendMessage({action:'isImageBanned', url:src}, function (result) {
if (!result) {
loadFullSizeImageTimeout = setTimeout(loadFullSizeImage, delay);
}
});
}

changing it back to how it was prior made it behave as expected for me:

hoverzoom/js/hoverzoom.js

Lines 1071 to 1074 in 6b9717e

// if the action key has been pressed over an image, no delay is applied
const delay = actionKeyDown || explicitCall ? 0 : (isVideoLink(srcDetails.url) ? options.displayDelayVideo : options.displayDelay);
loadFullSizeImageTimeout = setTimeout(loadFullSizeImage, delay);

While it's not from my commit, I will work on a fix for it.

@extesy
Copy link
Owner

extesy commented Dec 31, 2024

Thank you for finding the root cause. I thought I bi-sected the problem to your commit, but I guess I made a mistake while doing that. Please accept my apologies!

@LiliaDoe LiliaDoe deleted the Bug-fix branch December 31, 2024 04:27
extesy added a commit that referenced this pull request Dec 31, 2024
…ge (#1516)

- This fixes what #1515 was trying to fix
- setLoadImage used to set loadHoveredImage within callback so timeout
can be set outside of callback. This makes clearTimeout use the correct
timeout ID
- There is probably a smarter way of fixing this, but this works

---------

Co-authored-by: Oleg Anashkin <github@oleg.anashkin.org>
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