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

Update functions.js ignore video previews #2384

Merged
merged 3 commits into from
Jun 18, 2024
Merged

Update functions.js ignore video previews #2384

merged 3 commits into from
Jun 18, 2024

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented Jun 16, 2024

@ImprovedTube
Copy link
Member

Yay! @raszpl

eventlisteners can apply to previews (we can count how many seconds it plays there too or apply volume if it is on or playback speed)

auto-pause /only one video playing can have a sub-option to include previews or not. (generally while a fix can might be the better for most common on the long term, many current user will be used to the current way now or learned to appreciate that)

@raszpl
Copy link
Contributor Author

raszpl commented Jun 17, 2024

Yay! @raszpl

eventlisteners can apply to previews (we can count how many seconds it plays there too or apply volume if it is on or playback speed)

'could' if someone rewrote them specifically for it, looking at few now they apply to main video only

auto-pause /only one video playing can have a sub-option to include previews or not. (generally while a fix can might be the better for most common on the long term, many current user will be used to the current way now or learned to appreciate that)

the fix fixes current way it was supposed to work. No user switches "Disable video playback on hover" expecting it to break "Pause while I watch a 2nd video" :)

@ImprovedTube ImprovedTube merged commit 3cb2844 into code-charity:master Jun 18, 2024
1 check passed
@ImprovedTube
Copy link
Member

hi! @raszpl! sorry, came from a different perspective context: While Disable video playback on hover doesn't seem to have an effect at mine currently,
our "Pause while I watch a 2nd video" alone should un-pause the 1st video, when pausing the 2nd one.
This doesn't seem to happen. Then it also can be relaxing, if the first video temporarily pauses while previewing search results or related videos. (and when using https://github.com/extesy/hoverzoom).
When that's working, we can add sub-options: Don't auto un-pause, when i stop watching a 2nd video & don't consider a hover-previews as a video.

# DISABLE VIDEO PLAYBACK ON HOVER
--------------------------------------------------------------*/
extension.features.disableThumbnailPlayback = function (event) {
if (event instanceof Event) {
if (event.composedPath().some(elem => (elem.matches != null && elem.matches('#content.ytd-rich-item-renderer, #contents.ytd-item-section-renderer'))
)) {
event.stopImmediatePropagation();
}
} else {
if (extension.storage.get('disable_thumbnail_playback') === true) {
window.addEventListener('mouseenter', this.disableThumbnailPlayback, true);
} else {
window.removeEventListener('mouseenter', this.disableThumbnailPlayback, true);
}
}

(The given storage.don't consider a hover-previews as a video === true,
we could only skip sending the play event ("it-play") if closest('#inline-preview-player')
document.dispatchEvent(new CustomEvent('it-play'));
}
} else {
document.dispatchEvent(new CustomEvent('it-play'));

document.addEventListener('it-play', function (event) {
var videos = document.querySelectorAll('video');
try {chrome.runtime.sendMessage({action: 'play'})}
)

(if that makes / will make any sense)

they apply to main video only

(hope you like the unrelated commits, re: #2356)

@raszpl raszpl deleted the patch-8 branch June 19, 2024 05:14
@raszpl
Copy link
Contributor Author

raszpl commented Jun 19, 2024

came from a different perspective

Yes. The way Im looking at it Video is a proper video with player controls et, and animated thumbnails are Previews. Two bugs about this behavior suggest this is prevailing opinion.

context: While Disable video playback on hover doesn't seem to have an effect at mine currently

I get video previews in /subscription , and with .44 installed Disable video playback on hover does work on those. Doesnt work on Search ones obviously as its pre #2383

, our "Pause while I watch a 2nd video" alone should un-pause the 1st video

well :] its called Pause, doesnt say anything about unpausing :-P and there is no sign in the code about that intention either

, when pausing the 2nd one. This doesn't seem to happen.

Arent you thinking about 'Auto-pause while im not in the tab'? That one does resume because it keeps track in playerAutopauseWhenSwitchingTabs this.played_before_blur

When that's working

"when thats implemented from scratch" :)

we can add sub-options: Don't auto un-pause, when i stop watching a 2nd video & don't consider a hover-previews as a video.

as is now Users are not considering hover-previews to be Videos as evidenced by bug reports :)

(The given storage.don't consider a hover-previews as a video === true,
we could only skip sending the play event ("it-play") if closest('#inline-preview-player')
(if that makes / will make any sense)

yes. That will require also sending 'paused' message so background.js can broadcast back "another-video-stopped-playing" to all the tabs.

b0024cd 69e301e autoplayDisable doesnt parse properly, no biggie
but playerOnPlay change just break signaling sending play event regardless if we are playing or not

else {ImprovedTube.autoplayDisable(this); } 
				ImprovedTube.autoplayDisable(this);

What are you trying to do here? :)

@ImprovedTube
Copy link
Member

well :] its called Pause, doesnt say anything about unpausing :-P and there is no sign in the code about that intention either
Arent you thinking about 'Auto-pause while im not in the tab'?
"when thats implemented from scratch" :)

Both come with while in the name (not if). (Adding context is not a judgement.)

we can add sub-options: Don't auto un-pause, when i stop watching a 2nd video & don't consider a hover-previews as a video.

as is now Users are not considering hover-previews to be Videos as evidenced by bug reports :)

(While not all users are the same)

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