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

Use the typed enum CefEventFlags for modifiers in keyboard events. #667

Merged
merged 2 commits into from
Dec 4, 2014

Conversation

jankurianski
Copy link
Member

Cef says for the modifier field:

See cef_event_flags_t for values.

So I think it is safe to cast the value to the existing CefEventFlags enum type.

With this PR, I am now able to use the following logic in my app:

        bool IKeyboardHandler.OnPreKeyEvent(IWebBrowser browser, KeyType type, int windowsKeyCode, int nativeKeyCode, CefEventFlags modifiers, bool isSystemKey, bool isKeyboardShortcut)
        {
                Keys keys = (Keys)windowsKeyCode;
                if ((modifiers & CefEventFlags.ControlDown) != 0)
                    keys |= Keys.Control;
                else if ((modifiers & CefEventFlags.AltDown) != 0)
                    keys |= Keys.Alt;
                else if ((modifiers & CefEventFlags.ShiftDown) != 0)
                    keys |= Keys.Shift;

@amaitland
Copy link
Member

Sounds reasonable 👍

@amaitland
Copy link
Member

One day I'd like to remove KeyTypeToManaged and just cast that directly to KeyType

https://github.com/cefsharp/CefSharp/blob/master/CefSharp.Core/Internals/ClientAdapter.cpp#L115

@amaitland amaitland added this to the 37.0.0 milestone Dec 4, 2014
@jankurianski
Copy link
Member Author

I wonder why it isn't a straight cast, as cef_key_event_type_t in cef_types.h sets KEYEVENT_RAWKEYDOWN = 0, so the other enum values have incrementing values that C# could use to convert. Just historical reasons?

@amaitland
Copy link
Member

Just historical reasons?

That would be my guess. It's one of those that I look at from time to time and think about changing and never do! lol

@jankurianski
Copy link
Member Author

I know what you mean 😄 I'll do a PR for it now since I'm in the area.

@amaitland
Copy link
Member

Thanks 👍

If it's easier, just push another commit to this branch, it's a trivial change.

@jankurianski
Copy link
Member Author

Done.

amaitland added a commit that referenced this pull request Dec 4, 2014
Use the typed enum CefEventFlags for modifiers in keyboard events.
@amaitland amaitland merged commit 2c4723e into cefsharp:master Dec 4, 2014
@amaitland
Copy link
Member

Done 😄

@jankurianski
Copy link
Member Author

Cheers 🍻

@amaitland
Copy link
Member

No probs 👍

I was planning on releasing another -pre version with the hopes of releasing a stable version of 37 sometime next week. Any other changes in the pipeline?

@jankurianski
Copy link
Member Author

Nope, nothing planned. Just needed to implement keyboard shortcut passing from Chomium -> ToolStrip menus today (e.g. so CTRL+S still fires our File | Save menu, even if the Chromium control has focus) and thought I'd submit this cleanup.

We run quite a few functional tests nightly (i.e. automated mouse/keyboard driving of our app, with screenshots between each step) and the recent CefSharp master builds have proven to be stable. So I think it's a good time for a -pre stable candidate.

@amaitland
Copy link
Member

We run quite a few functional tests nightly (i.e. automated mouse/keyboard driving of our app, with screenshots between each step) and the recent CefSharp master builds have proven to be stable. So I think it's a good time for a -pre stable candidate.

Greatly appreciate the feedback 😄 Merged a few bug fixes today, so if you notice anything stability wise if you should let me know that would be great 👍

@numbersint numbersint deleted the feature/event-flags branch December 22, 2014 11:32
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.

2 participants