Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

handle the function keys by preventing the default action, fixes #1367 #1373

Closed
wants to merge 4 commits into from
Closed

handle the function keys by preventing the default action, fixes #1367 #1373

wants to merge 4 commits into from

Conversation

clarkbw
Copy link
Contributor

@clarkbw clarkbw commented Dec 2, 2016

Associated Issue: #1367

Summary of Changes

  • add preventDefault and stopPropogation

Test Plan (tested in Windows only)

  • Pause on breakpoint press F10 to step over

Copy link
Contributor

@jasonLaster jasonLaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid stop propagation as we're listening on the window? Not much higher to go!

Also I wonder if you could add a helper

stopPropogation(() => this.stepIn())

Sorry, typing on the go.

@clarkbw
Copy link
Contributor Author

clarkbw commented Dec 8, 2016

Can we avoid stop propagation as we're listening on the window? Not much higher to go!

I tried this but I believe if you don't stop propagation the event is bubbled out of the window context because this is a key the browser / OS handles. I tried using just one or the other and it required both to be used in order to stop the default action from happening.

Also I wonder if you could add a helper

stopPropogation(() => this.stepIn())

Sounds like a good plan. Will update soon.

shortcuts.on(`${ctrlKey}Shift+F11`, this.props.stepOut);
const handleEvent = (e, func) => {
e.preventDefault();
if (e.stopPropogation) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strange, i've never seen this stopPropogation not present. What case are you handling here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was seeing an error in the console complaining that stopPropogation didn't exist while working on my Mac and I wasn't see on Windows. I'll be able to check it out on both this week some time. Not sure what's really going on here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, i can take a look

@jasonLaster
Copy link
Contributor

I dropped the if because it's part of the DOM spec lol. if we break anything we can fix it :)

https://developer.mozilla.org/en-US/docs/Web/API/Event/stopPropagation

Copy link
Contributor

@jasonLaster jasonLaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jasonLaster
Copy link
Contributor

closing this in favor of #1455

@clarkbw clarkbw deleted the function-keys-#1367 branch December 16, 2016 18:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants