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 #68849 "Screencast: Some keys do not get special treatment" #69904

wants to merge 3 commits into
base: master


None yet
3 participants
Copy link

commented Mar 6, 2019

This pull request fixes issue #68849

@dyc3 dyc3 force-pushed the dyc3:fix-screencast-keys-68849 branch from 5bbf44e to e660bd1 Mar 6, 2019

@@ -193,19 +196,24 @@ export class ToggleScreencastModeAction extends Action {
const label = keybinding.getLabel();

if (!event.ctrlKey && !event.altKey && !event.metaKey && !event.shiftKey && this.keybindingService.mightProducePrintableCharacter(event) && label) {
keyboardMarker.textContent += ' ' + label;
if (previousKeyCode !== event.keyCode || !(event.keyCode === KeyCode.Backspace || event.keyCode === KeyCode.Escape)) {

This comment has been minimized.

Copy link

adamtajti Mar 6, 2019


I think it would be enough to just add

&& !(event.keyCode === KeyCode.Backspace || event.keyCode === KeyCode.Escape)

to the if at line 198 and then we can drop the logic built around previousKeyCode.

This comment has been minimized.

Copy link

dyc3 Mar 6, 2019

Author Contributor

This does slightly change the behavior, but I think this would be better.

@dyc3 dyc3 force-pushed the dyc3:fix-screencast-keys-68849 branch from e660bd1 to 7eede97 Mar 6, 2019

Copy link

left a comment

I think it's good 👍 Approved from my end

@joaomoreno joaomoreno self-assigned this Mar 7, 2019

@joaomoreno joaomoreno added this to the Backlog milestone Mar 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.