Fixed keyboard shortcuts not triggering early-listeners

[gurupras]

Link to related issue (if applicable)

#1345

Summary of proposed changes

Replaced direct calls with proxy() to ensure that default handlers are called first.

Here's a pen with the fix

Checklist

• Use develop as the base branch
• Exclude the gulp build (/dist changes) from the PR
• Test on supported browsers

[sampotts]

Small fix needed 👍

[sampotts]

This should toggle play

[gurupras]

Sorry, could you elaborate? I don't quite understand what I should do about that line.
Should the 'play' become 'togglePlay'?

[sampotts]

Sorry for mega (!!) late response but I think it should have some logic that calls 'play' or 'pause' depending on current state...

[gurupras]

I can do this, but I don't think the 'pause' event is triggered anywhere else in the code.

plyr/src/js/listeners.js

Lines 539 to 544 in 977a839

	// Play/pause toggle
	if (elements.buttons.play) {
	Array.from(elements.buttons.play).forEach(button => {
	this.bind(button, 'click', player.togglePlay, 'play');
	});
	}

Clicking the play/pause buttons only triggers the 'play' event.

[gurupras]

@sampotts, let me know how to fix this when you have some time and I'll fix it so it can be merged in :)

[gurupras]

Will update the code. Thanks for pointing stuff out.

…

On Mon, Jan 13, 2020, 4:24 PM Sam Potts ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/js/listeners.js <#1346 (comment)>: > break; case 67: // C key if (!repeat) { - player.toggleCaptions(); + this.proxy(event, () => player.toggleCaptions(), 'captions') As above ------------------------------ In src/js/listeners.js <#1346 (comment)>: > @@ -166,7 +173,7 @@ class Listeners { // Escape is handle natively when in full screen // So we only need to worry about non native if (code === 27 && !player.fullscreen.usingNative && player.fullscreen.active) { - player.fullscreen.toggle(); + this.proxy(event, () => player.fullscreen.toggle(), 'fullscreen') As above — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1346?email_source=notifications&email_token=AAWMDV3R4J6XBNMFR2BX7ITQ5TLZVA5CNFSM4GXXQSO2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCRSMLVI#pullrequestreview-342148565>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAWMDV4RDXGJ2I5WY43AWKTQ5TLZVANCNFSM4GXXQSOQ> .

[sampotts]

Will update the code. Thanks for pointing stuff out.

Cheers mate 👍