Skip to content

Commit

Permalink
Fix displayed ordering of keyboard shortcuts
Browse files Browse the repository at this point in the history
  • Loading branch information
Keavon committed Feb 29, 2024
1 parent 96e52c1 commit 4405e01
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 38 deletions.
36 changes: 16 additions & 20 deletions editor/src/messages/input_mapper/default_mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ pub fn default_mapping() -> Mapping {
entry!(KeyDown(Minus); action_dispatch=TransformLayerMessage::TypeNegate),
entry!(KeyDown(Comma); action_dispatch=TransformLayerMessage::TypeDecimalPoint),
entry!(KeyDown(Period); action_dispatch=TransformLayerMessage::TypeDecimalPoint),
entry!(PointerMove; refresh_keys=[Shift, Control], action_dispatch=TransformLayerMessage::PointerMove { slow_key: Shift, snap_key: Control }),
entry!(PointerMove; refresh_keys=[Control, Shift], action_dispatch=TransformLayerMessage::PointerMove { slow_key: Shift, snap_key: Control }),
//
// SelectToolMessage
entry!(PointerMove; refresh_keys=[Control, Shift, Alt], action_dispatch=SelectToolMessage::PointerMove { axis_align: Shift, snap_angle: Control, center: Alt, duplicate: Alt }),
entry!(PointerMove; refresh_keys=[Control, Alt, Shift], action_dispatch=SelectToolMessage::PointerMove { axis_align: Shift, snap_angle: Control, center: Alt, duplicate: Alt }),
entry!(KeyDown(Lmb); action_dispatch=SelectToolMessage::DragStart { add_to_selection: Shift, select_deepest: Accel }),
entry!(KeyUp(Lmb); action_dispatch=SelectToolMessage::DragStop { remove_from_selection: Shift }),
entry!(KeyDown(Enter); action_dispatch=SelectToolMessage::Enter),
Expand Down Expand Up @@ -174,7 +174,7 @@ pub fn default_mapping() -> Mapping {
entry!(KeyUp(Lmb); action_dispatch=LineToolMessage::DragStop),
entry!(KeyDown(Rmb); action_dispatch=LineToolMessage::Abort),
entry!(KeyDown(Escape); action_dispatch=LineToolMessage::Abort),
entry!(PointerMove; refresh_keys=[Alt, Control, Shift], action_dispatch=LineToolMessage::PointerMove { center: Alt, lock_angle: Control, snap_angle: Shift }),
entry!(PointerMove; refresh_keys=[Control, Alt, Shift], action_dispatch=LineToolMessage::PointerMove { center: Alt, lock_angle: Control, snap_angle: Shift }),
//
// PathToolMessage
entry!(KeyDown(Delete); modifiers=[Accel], action_dispatch=PathToolMessage::DeleteAndBreakPath),
Expand Down Expand Up @@ -222,7 +222,7 @@ pub fn default_mapping() -> Mapping {
entry!(KeyDown(ArrowDown); modifiers=[Shift, ArrowRight], action_dispatch=PathToolMessage::NudgeSelectedPoints { delta_x: BIG_NUDGE_AMOUNT, delta_y: BIG_NUDGE_AMOUNT }),
//
// PenToolMessage
entry!(PointerMove; refresh_keys=[Shift, Control], action_dispatch=PenToolMessage::PointerMove { snap_angle: Shift, break_handle: Alt, lock_angle: Control}),
entry!(PointerMove; refresh_keys=[Control, Shift], action_dispatch=PenToolMessage::PointerMove { snap_angle: Shift, break_handle: Alt, lock_angle: Control}),
entry!(KeyDown(Lmb); action_dispatch=PenToolMessage::DragStart),
entry!(KeyUp(Lmb); action_dispatch=PenToolMessage::DragStop),
entry!(KeyDown(Rmb); action_dispatch=PenToolMessage::Confirm),
Expand Down Expand Up @@ -268,7 +268,7 @@ pub fn default_mapping() -> Mapping {
entry!(KeyDown(KeyE); action_dispatch=ToolMessage::ActivateToolEllipse),
entry!(KeyDown(KeyY); action_dispatch=ToolMessage::ActivateToolPolygon),
entry!(KeyDown(KeyB); action_dispatch=ToolMessage::ActivateToolBrush),
entry!(KeyDown(KeyX); modifiers=[Shift, Accel], action_dispatch=ToolMessage::ResetColors),
entry!(KeyDown(KeyX); modifiers=[Accel, Shift], action_dispatch=ToolMessage::ResetColors),
entry!(KeyDown(KeyX); modifiers=[Shift], action_dispatch=ToolMessage::SwapColors),
entry!(KeyDown(KeyC); modifiers=[Alt], action_dispatch=ToolMessage::SelectRandomPrimaryColor),
//
Expand Down Expand Up @@ -325,6 +325,16 @@ pub fn default_mapping() -> Mapping {
entry!(KeyDown(KeyG); action_dispatch=TransformLayerMessage::BeginGrab),
entry!(KeyDown(KeyR); action_dispatch=TransformLayerMessage::BeginRotate),
entry!(KeyDown(KeyS); action_dispatch=TransformLayerMessage::BeginScale),
entry!(KeyDown(Digit0); action_dispatch=TransformLayerMessage::TypeDigit { digit: 0 }),
entry!(KeyDown(Digit1); action_dispatch=TransformLayerMessage::TypeDigit { digit: 1 }),
entry!(KeyDown(Digit2); action_dispatch=TransformLayerMessage::TypeDigit { digit: 2 }),
entry!(KeyDown(Digit3); action_dispatch=TransformLayerMessage::TypeDigit { digit: 3 }),
entry!(KeyDown(Digit4); action_dispatch=TransformLayerMessage::TypeDigit { digit: 4 }),
entry!(KeyDown(Digit5); action_dispatch=TransformLayerMessage::TypeDigit { digit: 5 }),
entry!(KeyDown(Digit6); action_dispatch=TransformLayerMessage::TypeDigit { digit: 6 }),
entry!(KeyDown(Digit7); action_dispatch=TransformLayerMessage::TypeDigit { digit: 7 }),
entry!(KeyDown(Digit8); action_dispatch=TransformLayerMessage::TypeDigit { digit: 8 }),
entry!(KeyDown(Digit9); action_dispatch=TransformLayerMessage::TypeDigit { digit: 9 }),
//
// NavigationMessage
entry!(KeyDown(Mmb); modifiers=[Alt], action_dispatch=NavigationMessage::RotateCanvasBegin { was_dispatched_from_menu: false }),
Expand Down Expand Up @@ -374,19 +384,6 @@ pub fn default_mapping() -> Mapping {
];
let (mut key_up, mut key_down, mut key_up_no_repeat, mut key_down_no_repeat, mut double_click, mut wheel_scroll, mut pointer_move) = mappings;

// TODO: Hardcode these 10 lines into 10 lines of declarations, or make this use a macro to do all 10 in one line
const NUMBER_KEYS: [Key; 10] = [Digit0, Digit1, Digit2, Digit3, Digit4, Digit5, Digit6, Digit7, Digit8, Digit9];
for (i, key) in NUMBER_KEYS.iter().enumerate() {
key_down[*key as usize].0.insert(
0,
MappingEntry {
action: TransformLayerMessage::TypeDigit { digit: i as u8 }.into(),
input: InputMapperMessage::KeyDown(*key),
modifiers: modifiers!(),
},
);
}

let sort = |list: &mut KeyMappingEntries| list.0.sort_by(|u, v| v.modifiers.ones().cmp(&u.modifiers.ones()));
for list in [&mut key_up, &mut key_down, &mut key_up_no_repeat, &mut key_down_no_repeat] {
for sublist in list {
Expand All @@ -410,9 +407,8 @@ pub fn default_mapping() -> Mapping {
}
}

/// Defaults except that scrolling without modifiers is bound to zooming instead of vertical panning
/// Default mappings except that scrolling without modifier keys held down is bound to zooming instead of vertical panning
pub fn zoom_with_scroll() -> Mapping {
// TODO(multisn8): for other keymaps this patterns might be useful
use InputMapperMessage::*;

let mut mapping = default_mapping();
Expand Down
27 changes: 20 additions & 7 deletions editor/src/messages/input_mapper/input_mapper_message_handler.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::utility_types::input_keyboard::KeysGroup;
use super::utility_types::misc::Mapping;
use crate::messages::input_mapper::utility_types::input_keyboard::{self, Key};
use crate::messages::portfolio::utility_types::KeyboardPlatformLayout;
use crate::messages::prelude::*;

use std::fmt::Write;
Expand Down Expand Up @@ -59,10 +60,17 @@ impl InputMapperMessageHandler {
// Filter for the desired message
let found_actions = all_mapping_entries.filter(|entry| entry.action.to_discriminant() == *action_to_find);

let keyboard_layout = || GLOBAL_PLATFORM.get().copied().unwrap_or_default().as_keyboard_platform_layout();
let platform_accel_key = match keyboard_layout() {
KeyboardPlatformLayout::Standard => Key::Control,
KeyboardPlatformLayout::Mac => Key::Command,
};

// Find the key combinations for all keymaps matching the desired action
assert!(std::mem::size_of::<usize>() >= std::mem::size_of::<Key>());
found_actions
.map(|entry| {
// Get the modifier keys for the entry (and convert them to Key)
let mut keys = entry
.modifiers
.iter()
Expand All @@ -76,6 +84,7 @@ impl InputMapperMessageHandler {
})
.collect::<Vec<_>>();

// Append the key button for the entry
match entry.input {
InputMapperMessage::KeyDown(key) => keys.push(key),
InputMapperMessage::KeyUp(key) => keys.push(key),
Expand All @@ -84,16 +93,20 @@ impl InputMapperMessageHandler {
_ => (),
}

keys.sort_by(|a, b| {
keys.sort_by(|&a, &b| {
// Order according to platform guidelines mentioned at https://ux.stackexchange.com/questions/58185/normative-ordering-for-modifier-key-combinations
const ORDER: [Key; 4] = [Key::Control, Key::Alt, Key::Shift, Key::Command];

match (ORDER.contains(a), ORDER.contains(b)) {
(true, true) => ORDER.iter().position(|key| key == a).unwrap().cmp(&ORDER.iter().position(|key| key == b).unwrap()),
(true, false) => std::cmp::Ordering::Less,
(false, true) => std::cmp::Ordering::Greater,
(false, false) => std::cmp::Ordering::Equal,
}
// Treat the `Accel` virtual key as the platform's accel key for sorting comparison purposes
let a = if a == Key::Accel { platform_accel_key } else { a };
let b = if b == Key::Accel { platform_accel_key } else { b };

// Find where the keys are in the order, or put them at the end if they're not found
let a = ORDER.iter().position(|&key| key == a).unwrap_or(ORDER.len());
let b = ORDER.iter().position(|&key| key == b).unwrap_or(ORDER.len());

// Compare the positions of both keys
a.cmp(&b)
});

KeysGroup(keys)
Expand Down
6 changes: 4 additions & 2 deletions editor/src/messages/layout/layout_message_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,9 @@ impl<F: Fn(&MessageDiscriminant) -> Vec<KeysGroup>> MessageHandler<LayoutMessage
// Resend that diff
self.send_diff(vec![diff], layout_target, responses, &action_input_mapping);
}
SendLayout { layout, layout_target } => self.diff_and_send_layout_to_frontend(layout_target, layout, responses, &action_input_mapping),
SendLayout { layout, layout_target } => {
self.diff_and_send_layout_to_frontend(layout_target, layout, responses, &action_input_mapping);
}
WidgetValueCommit { layout_target, widget_id, value } => {
self.handle_widget_callback(layout_target, widget_id, value, WidgetValueAction::Commit, responses);
}
Expand Down Expand Up @@ -345,7 +347,7 @@ impl LayoutMessageHandler {
/// Send a diff to the frontend based on the layout target.
#[remain::check]
fn send_diff(&self, mut diff: Vec<WidgetDiff>, layout_target: LayoutTarget, responses: &mut VecDeque<Message>, action_input_mapping: &impl Fn(&MessageDiscriminant) -> Vec<KeysGroup>) {
diff.iter_mut().for_each(|diff| diff.new_value.apply_shortcut(action_input_mapping));
diff.iter_mut().for_each(|diff| diff.new_value.apply_keyboard_shortcut(action_input_mapping));

trace!("{layout_target:?} diff {diff:#?}");

Expand Down
4 changes: 2 additions & 2 deletions editor/src/messages/layout/utility_types/layout_widget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,8 +528,8 @@ pub enum DiffUpdate {
}

impl DiffUpdate {
/// Append the shortcut to the tooltip where applicable
pub fn apply_shortcut(&mut self, action_input_mapping: &impl Fn(&MessageDiscriminant) -> Vec<KeysGroup>) {
/// Append the keyboard shortcut to the tooltip where applicable
pub fn apply_keyboard_shortcut(&mut self, action_input_mapping: &impl Fn(&MessageDiscriminant) -> Vec<KeysGroup>) {
// Function used multiple times later in this code block to convert `ActionKeys::Action` to `ActionKeys::Keys` and append its shortcut to the tooltip
let apply_shortcut_to_tooltip = |tooltip_shortcut: &mut ActionKeys, tooltip: &mut String| {
let shortcut_text = tooltip_shortcut.to_keys(action_input_mapping);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ pub struct PopoverButton {

#[derive(Clone, Serialize, Deserialize, Derivative, Default, WidgetBuilder, specta::Type)]
#[derivative(Debug, PartialEq)]
#[serde(rename_all = "camelCase")]
pub struct ParameterExposeButton {
pub exposed: bool,

Expand All @@ -91,7 +90,6 @@ pub struct ParameterExposeButton {

#[derive(Clone, Serialize, Deserialize, Derivative, Default, WidgetBuilder, specta::Type)]
#[derivative(Debug, PartialEq)]
#[serde(rename_all = "camelCase")]
pub struct TextButton {
#[widget_builder(constructor)]
pub label: String,
Expand Down Expand Up @@ -160,7 +158,6 @@ pub struct ColorButton {

#[derive(Clone, Serialize, Deserialize, Derivative, Default, WidgetBuilder, specta::Type)]
#[derivative(Debug, PartialEq)]
#[serde(rename_all = "camelCase")]
pub struct BreadcrumbTrailButtons {
#[widget_builder(constructor)]
pub labels: Vec<String>,
Expand Down
2 changes: 1 addition & 1 deletion editor/src/messages/portfolio/utility_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ impl Platform {
pub fn as_keyboard_platform_layout(&self) -> KeyboardPlatformLayout {
match self {
Platform::Mac => KeyboardPlatformLayout::Mac,
Platform::Windows | Platform::Linux => KeyboardPlatformLayout::Standard,
Platform::Unknown => {
warn!("The platform has not been set, remember to send `GlobalsMessage::SetPlatform` during editor initialization.");
KeyboardPlatformLayout::Standard
}
_ => KeyboardPlatformLayout::Standard,
}
}
}
Expand Down
17 changes: 14 additions & 3 deletions frontend/src/components/widgets/labels/UserInputLabel.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,24 @@
<style lang="scss" global>
.user-input-label {
flex: 0 0 auto;
height: 100%;
align-items: center;
white-space: nowrap;
&.text-only {
.input-key + .input-key::before {
content: "+";
display: flex;
.input-key {
display: flex;
align-items: center;
.icon-label {
margin: calc(calc(18px - 12px) / 2) 0;
}
& + .input-key::before {
line-height: 18px;
content: "+";
}
}
}
Expand Down

0 comments on commit 4405e01

Please sign in to comment.