Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Decrease Font Size keyboard shortcut throws error if command is disabled #3030

Closed
lkcampbell opened this issue Mar 3, 2013 · 9 comments
Closed

Comments

@lkcampbell
Copy link
Contributor

Background:
I discovered this problem while working on another bug fix for adjusting font sizes. See the discussion at #3027.

Repro Steps:

  1. Patch Brackets with the pull request at Persisting font size between Brackets Sessions #3027 or just wait for the pull request to be merged (hopefully).
  2. Make sure all documents in the project are closed.
  3. Open the View Menu and observe that Increase Font Size, Decrease Font Size, and Restore Font Size commands are all disabled.
  4. Hit F12 to launch the Dev Tools and open the console.
  5. Press Ctrl-- (the shortcut key for Decrease Font Size command).

Observed Behavior:
A message is printed in the Dev Tools console: "Uncaught TypeError: Cannot call method 'state' of undefined."

Expected Behavior:
Nothing should happen. The shortcut keys for Increase Font Size (Ctrl-+) and Restore Font Size (Ctrl-0) do nothing. The shortcut key for Decrease Font Size shoud act the same.

@RaymondLim
Copy link
Contributor

@lkcampbell -- I think you can fix it by removing its displayKey (line 161 in base-config\keyboard.json) which is using unicode minus character as its shortcut. And I would like to know whether the issue is on Windows only or or Mac only or both platforms. We can consider on fixing it in native shell if we decide to keep using unicode minus character.

@WebsiteDeveloper
Copy link
Contributor

for me on windows i get errors for all three shortcuts when no document is open and i found a quick fix for the errors because the Command execute function returns undefined if the Command is disabled i fixed this by adding

return (new $.Deferred()).reject().promise()

@lkcampbell
Copy link
Contributor Author

@RaymondLim:

I'm on Windows 7 and the problem does go away when I edit keyboard.json per your suggestion.

@WebsiteDeveloper:

I'm not seeing the problem with the other shortcuts.

@RaymondLim
Copy link
Contributor

@WebsiteDeveloper:

I don't see any issue with the other two as well.

@lkcampbell:

I'm seeing a new issue after removing displayKey. Now Decrease font size is also showing its shortcut as Ctrl++ (same as Increase font size). So we have to fix the issue in native shell (at least on Windows).

@WebsiteDeveloper
Copy link
Contributor

this is weird i merged you branch in the current master to test it and still get the errors i'll try to merge again to see if i made any merge errors or something like that
By the way i'm also on windows 7

@WebsiteDeveloper
Copy link
Contributor

it seems that i get this additional errors because of my german keyboard layout where i have 0 + - 2times, on the Num block they all throw an error, and the other ones only the Ctrl-- logs an error.

@RaymondLim
Copy link
Contributor

@WebsiteDeveloper:

Nice discovery! I can reproduce it with numpad; so it has nothing to do with German keyboard layout. And your solution to fix it from CommandManager execute method is the right way to go. My suggestion of removing the displayKey won't fix the issue if the user uses keys from numpad. So please go ahead and fix it. I'll review your pull request when you submit it.

@RaymondLim
Copy link
Contributor

FBNC to @lkcampbell

@lkcampbell
Copy link
Contributor Author

Confirmed fix. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants