Merge pull request #1049 from linebender/keyboard

Keyboard event rework

CHANGELOG.md

@@ -13,6 +13,7 @@ You can find its changes [documented below](#060---2020-06-01).
 
 - `Image` and `ImageData` exported by default. ([#1011] by [@covercash2])
 - `Scale::from_scale` to `Scale::new`, and `Scale` methods `scale_x` / `scale_y` to `x` / `y`. ([#1042] by [@xStrom])
+- Major rework of keyboard event handling ([#1049] by [@raphlinus])
 - `Container::rounded` takes `KeyOrValue<f64>` instead of `f64`. ([#1054] by [@binomial0])
 
 ### Deprecated
@@ -235,7 +236,11 @@ Last release without a changelog :(
 [@jrmuizel]: https://github.com/jrmuizel
 [@scholtzan]: https://github.com/scholtzan
 [@covercash2]: https://github.com/covercash2
+<<<<<<< HEAD
+[@raphlinus]: https://github.com/raphlinus
+=======
 [@binomial0]: https://github.com/binomial0
+>>>>>>> master
 
 [#599]: https://github.com/linebender/druid/pull/599
 [#611]: https://github.com/linebender/druid/pull/611
@@ -339,6 +344,7 @@ Last release without a changelog :(
 [#1028]: https://github.com/linebender/druid/pull/1028
 [#1042]: https://github.com/linebender/druid/pull/1042
 [#1043]: https://github.com/linebender/druid/pull/1043
+[#1049]: https://github.com/linebender/druid/pull/1049
 [#1050]: https://github.com/linebender/druid/pull/1050
 [#1054]: https://github.com/linebender/druid/pull/1054
 [#1062]: https://github.com/linebender/druid/pull/1062

docs/book_examples/src/custom_widgets_md.rs

@@ -1,7 +1,7 @@
+use druid::keyboard_types::Key;
 use druid::widget::{Controller, Label, Painter, SizedBox, TextBox};
 use druid::{
-    Color, Env, Event, EventCtx, KeyCode, PaintCtx, RenderContext, Selector, TimerToken, Widget,
-    WidgetExt,
+    Color, Env, Event, EventCtx, PaintCtx, RenderContext, Selector, TimerToken, Widget, WidgetExt,
 };
 use std::time::Duration;
 
@@ -59,10 +59,10 @@ impl Controller<String, TextBox> for TextBoxActionController {
         env: &Env,
     ) {
         match event {
-            Event::KeyDown(k) if k.key_code == KeyCode::Return => {
+            Event::KeyDown(k) if k.key == Key::Enter => {
                 ctx.submit_command(ACTION, None);
             }
-            Event::KeyUp(k) if k.key_code != KeyCode::Return => {
+            Event::KeyUp(k) if k.key == Key::Enter => {
                 self.timer = Some(ctx.request_timer(DELAY));
                 child.event(ctx, event, data, env);
             }

druid-shell/Cargo.toml

@@ -28,6 +28,7 @@ time = "0.2.16"
 cfg-if = "0.1.10"
 instant = { version = "0.1.4", features = ["wasm-bindgen"] }
 anyhow = "1.0.31"
+keyboard-types = { version = "0.5.0", default_features = false }
 
 # Optional dependencies
 cairo-rs = { version = "0.8.1", default_features = false, optional = true }

druid-shell/examples/shello.rs

@@ -63,13 +63,14 @@ impl WinHandler for HelloState {
     }
 
     fn key_down(&mut self, event: KeyEvent) -> bool {
-        let deadline = std::time::Duration::from_millis(500);
-        let id = self.handle.request_timer(deadline);
-        println!("keydown: {:?}, timer id = {:?}", event, id);
-
+        println!("keydown: {:?}", event);
         false
     }
 
+    fn key_up(&mut self, event: KeyEvent) {
+        println!("keyup: {:?}", event);
+    }
+
     fn wheel(&mut self, event: &MouseEvent) {
         println!("mouse_wheel {:?}", event);
     }

druid-shell/src/hotkey.rs

@@ -18,8 +18,9 @@ use std::borrow::Borrow;
 
 use log::warn;
 
-use crate::keyboard::{KeyEvent, KeyModifiers};
-use crate::keycodes::KeyCode;
+use crate::{IntoKey, KbKey, KeyEvent, Modifiers};
+
+// TODO: fix docstring
 
 /// A description of a keyboard shortcut.
 ///
@@ -31,40 +32,33 @@ use crate::keycodes::KeyCode;
 /// [`SysMods`] matches the Command key on macOS and Ctrl elsewhere:
 ///
 /// ```
-/// use druid_shell::{HotKey, KeyEvent, KeyCode, RawMods, SysMods};
+/// use druid_shell::{HotKey, KbKey, KeyEvent, RawMods, SysMods};
 ///
 /// let hotkey = HotKey::new(SysMods::Cmd, "a");
 ///
 /// #[cfg(target_os = "macos")]
-/// assert!(hotkey.matches(KeyEvent::for_test(RawMods::Meta, "a", KeyCode::KeyA)));
+/// assert!(hotkey.matches(KeyEvent::for_test(RawMods::Meta, "a")));
 ///
 /// #[cfg(target_os = "windows")]
-/// assert!(hotkey.matches(KeyEvent::for_test(RawMods::Ctrl, "a", KeyCode::KeyA)));
+/// assert!(hotkey.matches(KeyEvent::for_test(RawMods::Ctrl, "a")));
 /// ```
 ///
 /// `None` matches only the key without modifiers:
 ///
 /// ```
-/// use druid_shell::{HotKey, KeyEvent, KeyCode, RawMods, SysMods};
+/// use druid_shell::{HotKey, KbKey, KeyEvent, RawMods, SysMods};
 ///
-/// let hotkey = HotKey::new(None, KeyCode::ArrowLeft);
+/// let hotkey = HotKey::new(None, KbKey::ArrowLeft);
 ///
-/// assert!(hotkey.matches(KeyEvent::for_test(RawMods::None, "", KeyCode::ArrowLeft)));
-/// assert!(!hotkey.matches(KeyEvent::for_test(RawMods::Ctrl, "", KeyCode::ArrowLeft)));
+/// assert!(hotkey.matches(KeyEvent::for_test(RawMods::None, KbKey::ArrowLeft)));
+/// assert!(!hotkey.matches(KeyEvent::for_test(RawMods::Ctrl, KbKey::ArrowLeft)));
 /// ```
 ///
 /// [`SysMods`]: enum.SysMods.html
 #[derive(Debug, Clone)]
 pub struct HotKey {
     pub(crate) mods: RawMods,
-    pub(crate) key: KeyCompare,
-}
-
-/// Something that can be compared with a keyboard key.
-#[derive(Debug, Clone, PartialEq)]
-pub enum KeyCompare {
-    Code(KeyCode),
-    Text(&'static str),
+    pub(crate) key: KbKey,
 }
 
 impl HotKey {
@@ -75,40 +69,34 @@ impl HotKey {
     /// 'Command' key on macOS with the 'Ctrl' key on other platforms.
     ///
     /// The second argument describes the non-modifier key. This can be either
-    /// a `&'static str` or a [`KeyCode`]. If it is a `&str`, it will be compared
-    /// against the [`unmodified text`] for a given key event; if it is a [`KeyCode`]
-    /// it will be compared against the event's key code.
-    ///
-    /// In general, [`KeyCode`] should be preferred for non-printing keys, like
-    /// the arrows or backspace.
+    /// a `&str` or a [`KbKey`]; the former is merely a convenient
+    /// shorthand for `KbKey::Character()`.
     ///
     /// # Examples
     /// ```
-    /// use druid_shell::{HotKey, KeyEvent, KeyCode, RawMods, SysMods};
+    /// use druid_shell::{HotKey, KbKey, RawMods, SysMods};
     ///
     /// let select_all = HotKey::new(SysMods::Cmd, "a");
-    /// let esc = HotKey::new(None, KeyCode::Escape);
+    /// let esc = HotKey::new(None, KbKey::Escape);
     /// let macos_fullscreen = HotKey::new(RawMods::CtrlMeta, "f");
     /// ```
     ///
-    /// [`KeyCode`]: enum.KeyCode.html
+    /// [`Key`]: keyboard_types::Key
     /// [`SysMods`]: enum.SysMods.html
     /// [`RawMods`]: enum.RawMods.html
-    /// [`unmodified text`]: struct.KeyEvent.html#method.unmod_text
-    pub fn new(mods: impl Into<Option<RawMods>>, key: impl Into<KeyCompare>) -> Self {
+    pub fn new(mods: impl Into<Option<RawMods>>, key: impl IntoKey) -> Self {
         HotKey {
             mods: mods.into().unwrap_or(RawMods::None),
-            key: key.into(),
+            key: key.into_key(),
         }
         .warn_if_needed()
     }
 
-    //TODO: figure out if we need to be normalizing case or something? This requires
-    //correctly documenting the expected behaviour of `unmod_text`.
+    //TODO: figure out if we need to be normalizing case or something?
     fn warn_if_needed(self) -> Self {
-        if let KeyCompare::Text(s) = self.key {
-            let km: KeyModifiers = self.mods.into();
-            if km.shift && s.chars().any(|c| c.is_uppercase()) {
+        if let KbKey::Character(s) = &self.key {
+            let km: Modifiers = self.mods.into();
+            if km.shift() && s.chars().any(|c| c.is_lowercase()) {
                 warn!(
                     "warning: HotKey {:?} includes shift, but text is lowercase. \
                      Text is matched literally; this may cause problems.",
@@ -119,16 +107,12 @@ impl HotKey {
         self
     }
 
-    /// Returns `true` if this [`KeyEvent`] matches this `HotKey`.
+    /// Returns `true` if this [`KeyboardEvent`] matches this `HotKey`.
     ///
-    /// [`KeyEvent`]: struct.KeyEvent.html
+    /// [`KeyboardEvent`]: keyboard_types::KeyEvent
     pub fn matches(&self, event: impl Borrow<KeyEvent>) -> bool {
         let event = event.borrow();
-        self.mods == event.mods
-            && match self.key {
-                KeyCompare::Code(code) => code == event.key_code,
-                KeyCompare::Text(text) => Some(text) == event.text(),
-            }
+        self.mods == event.mods && self.key == event.key
     }
 }
 
@@ -153,7 +137,7 @@ pub enum SysMods {
 //TODO: should something like this just _replace_ keymodifiers?
 /// A representation of the active modifier keys.
 ///
-/// This is intended to be clearer than `KeyModifiers`, when describing hotkeys.
+/// This is intended to be clearer than `Modifiers`, when describing hotkeys.
 #[derive(Debug, Clone, Copy)]
 pub enum RawMods {
     None,
@@ -174,35 +158,35 @@ pub enum RawMods {
     AltCtrlMetaShift,
 }
 
-impl std::cmp::PartialEq<KeyModifiers> for RawMods {
-    fn eq(&self, other: &KeyModifiers) -> bool {
-        let mods: KeyModifiers = (*self).into();
+impl std::cmp::PartialEq<Modifiers> for RawMods {
+    fn eq(&self, other: &Modifiers) -> bool {
+        let mods: Modifiers = (*self).into();
         mods == *other
     }
 }
 
-impl std::cmp::PartialEq<RawMods> for KeyModifiers {
+impl std::cmp::PartialEq<RawMods> for Modifiers {
     fn eq(&self, other: &RawMods) -> bool {
         other == self
     }
 }
 
-impl std::cmp::PartialEq<KeyModifiers> for SysMods {
-    fn eq(&self, other: &KeyModifiers) -> bool {
+impl std::cmp::PartialEq<Modifiers> for SysMods {
+    fn eq(&self, other: &Modifiers) -> bool {
         let mods: RawMods = (*self).into();
         mods == *other
     }
 }
 
-impl std::cmp::PartialEq<SysMods> for KeyModifiers {
+impl std::cmp::PartialEq<SysMods> for Modifiers {
     fn eq(&self, other: &SysMods) -> bool {
         let other: RawMods = (*other).into();
         &other == self
     }
 }
 
-impl From<RawMods> for KeyModifiers {
-    fn from(src: RawMods) -> KeyModifiers {
+impl From<RawMods> for Modifiers {
+    fn from(src: RawMods) -> Modifiers {
         let (alt, ctrl, meta, shift) = match src {
             RawMods::None => (false, false, false, false),
             RawMods::Alt => (true, false, false, false),
@@ -221,12 +205,12 @@ impl From<RawMods> for KeyModifiers {
             RawMods::CtrlMetaShift => (false, true, true, true),
             RawMods::AltCtrlMetaShift => (true, true, true, true),
         };
-        KeyModifiers {
-            alt,
-            ctrl,
-            meta,
-            shift,
-        }
+        let mut mods = Modifiers::empty();
+        mods.set(Modifiers::ALT, alt);
+        mods.set(Modifiers::CONTROL, ctrl);
+        mods.set(Modifiers::META, meta);
+        mods.set(Modifiers::SHIFT, shift);
+        mods
     }
 }
 
@@ -259,15 +243,3 @@ impl From<SysMods> for RawMods {
         }
     }
 }
-
-impl From<KeyCode> for KeyCompare {
-    fn from(src: KeyCode) -> KeyCompare {
-        KeyCompare::Code(src)
-    }
-}
-
-impl From<&'static str> for KeyCompare {
-    fn from(src: &'static str) -> KeyCompare {
-        KeyCompare::Text(src)
-    }
-}