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 for polish programmers keyboard alt+z #580

Merged
merged 1 commit into from Apr 1, 2015
Merged

fix for polish programmers keyboard alt+z #580

merged 1 commit into from Apr 1, 2015

Conversation

akupiec
Copy link
Contributor

@akupiec akupiec commented Mar 17, 2015

Fix for bug mentioned here #518

@SimeonC
Copy link
Collaborator

SimeonC commented Mar 17, 2015

Thanks for the fix, I'll have to check this through as it seems some of my tests using ctrl don't work.

@akupiec
Copy link
Contributor Author

akupiec commented Mar 19, 2015

I ran your tests (grunt karma) on my local machine and they all passed without problems, so I can't do much about it.

@SimeonC
Copy link
Collaborator

SimeonC commented Mar 19, 2015

Did you run just grunt karma or the just grunt? You should just run the default grunt task that then compiles everything up before the tests. (You can see the tests run by travis here https://travis-ci.org/fraywing/textAngular/builds/54745052)

Don't worry about it this time, I know the issue and can fix it up, it'll just take me a bit to get some time to do it as I'm starting a new job soon so chaos reigns at the moment!

@SimeonC
Copy link
Collaborator

SimeonC commented Apr 1, 2015

Having a look at it, I think your code will actually break things as in the case of CTRL+Z the ctrl flag will be false as the keycode will be for ctrl or z not both we can't undo by key command.

I think the correct fix is this: if(!event.altKey && (event.metaKey || ctrlKey)){ which I got from here - I think last time I didn't bracket correctly.

I've also added in a test to check this time - Triggering Alt + Ctrl + Z doesn't fire the undo command now which from what I understand is the problem. Can you grab the version from master and confirm this please?

@SimeonC SimeonC merged commit b2c7886 into textAngular:master Apr 1, 2015
@akupiec
Copy link
Contributor Author

akupiec commented Apr 8, 2015

@SimeonC
Thank you very much, ctrl+z as well polish "ż" works now fine.
Please don't forget about release ^_^

@SimeonC
Copy link
Collaborator

SimeonC commented Apr 8, 2015

Awesome, thanks for confirming it - I couldn't myself.

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.

None yet

2 participants