forked from igrigorik/videospeed
-
Notifications
You must be signed in to change notification settings - Fork 55
Seperate lowercase and uppercase input #168
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
Closed
magnus-ISU
wants to merge
25
commits into
codebicycle:firefox-port
from
magnus-ISU:seperate-lower-upper
Closed
Seperate lowercase and uppercase input #168
magnus-ISU
wants to merge
25
commits into
codebicycle:firefox-port
from
magnus-ISU:seperate-lower-upper
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…want to address before this is merged.
…t the code they are documenting
…ng wrong, but still doesn't work
…icated and should be essentially hidden, it is dumb to bypass the formatter
…ally better than keeping the first line right after
…s rule matches any reddit
… merge with seperate-lower-upper because it changes a lot about settings too
Open
Author
|
Just found out from reading #153 that this tracks upstream very closely and thus probably won't merge this. Time to plan to rewrite everything for chromium lol. Also upstream is at 7.1 already, hurry up and update this! |
Author
|
Upstream PR: igrigorik#907 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This refactors a lot of input code to resolve #167 (shortcuts can be set to either uppercase or lowercase). The way it does so is by mostly ignoring the
keyCodeattribute of an input event and instead using the stringkeywhich contains more information and in a more human-readable way. Unfortunately, this currently means it breaks any preconfigured options a user has set and users will need to reconfigure it. Assuming this is unacceptable (I assume it is) I can add in a migration strategy; but I need to know how best that done.I also removed the old 5.3.0 settings conversion code because its been here long enough I think. In doing so, I was able to simplify some logic. Additionally,
vine.codoes not really exist anymore so disabling the extension there by default is unneeded I think.I need testing for numpad keys to see if they still work the same. I wrote this all on a laptop without them.
There are several TODOs marked around the code where I wasn't sure what was going on and need review to tell me if my comments are correct or not.
The version is not bumped, please tell me how to change it. This could be a bump to
0.7.0.0or something, to signify it is a breaking change and add no migration. Or it could need to be updated to have a migration path, and become0.6.4.0. Or maybe the project tracks upstream versions directly and onlymanifest-versionshould be bumped. I'm not sure.EDIT: now adds the feature suggested in #119 also