-
Notifications
You must be signed in to change notification settings - Fork 26
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
Formally define builtin dvui control keybindings #125
Comments
I can see the appeal from a pure flexibility standpoint, but I'm trying to understand a concrete example of an app that would want to override the "standard" keyboard shortcuts. Usually the UI experts say to never change the expected keyboard shortcuts. I'm hesitant to define too much application-level behavior, but sometimes it's correct. Obviously I'm being wishy-washy here. For the zoom-in/out keybindings, I think they should be added to ScaleWidget, the only downside there is if focus is not in a child of ScaleWidget it wouldn't detect them. I'm also not entirely sure what the zooming algorithm should be - on hidpi screens like phones, arbitrary zooming works okay, but on lowdpi screens you want to snap to an integer font size. |
(Note, somehow I managed to accidentally edit your comment. It should be fixed now)
Just off the top of my head, here are a couple examples of applications that might want to disable the default keybindings
The idea isn't for applications to necessarily change the defaults, but rather, let application users optionally customize the keybindings according to thier preferences. (The organization of all dvui keybindings into one struct also has the benefit of providing a handy list of what all the default dvui keybindings even are) But maybe I am overcomplicating things here. This was just a thought I had so I wanted to discuss it |
Okay - I'm convinced. Let me think about the api and get back to you. |
I took a swing at this. I realized while finishing up #108 that the web backend needs the keybindings to be separated from the target (which for web is wasm32). Next up is plumbing the web so it has the right keybindings on mac/windows. How does it look? In particular:
|
Initial thoughts
Is there any particular reason you are storing "actions" as a string in a hashmap rather than using an enum? It seems like all these actions need to be comptime known anyways so an enum would be faster and simpler. An enum would also you eliminate the const Action = enum {
select,
copy,
paste,
//...
};
const Keybindings = struct {
bindings: [@typeInfo(Action).Enum.fields.len]std.ArrayList(dvui.Key),
pub fn matches(self: @This(), key: dvui.Key, action: Action) bool {
const possible_keys = self.bindings.items[@intFromEnum(action)];
for(possible_keys) |possible| {
if(std.meta.eql(possible, key)) return true;
}
return false;
}
pub fn addBindings(self: *@This(), bindings: []const Binding) !void {
for(bindings) |binding| {
try self.bindings[@intFromEnum(binding.action)].append(binding.key);
}
}
};
const Binding = struct {
action: Action,
key: dvui.Key,
};
const mac_text_keybindings = [_]Binding{
.{
.action = Action.copy,
.key = Key{},
}, .{
.action = Action.copy,
.key = Key{},
},
//...
};
I think calling this key |
I thought about a non-exhaustive enum, but is there a game or app that would want to define more keybinds at runtime? I can think of some use cases for that (like a photo editor with tons of plugins and you can define a keybind to run one of them). Unsure if that is common enough to warrant supporting easily. A non-exhaustive enum means the app would deal with numbers instead of strings, so any naming would happen separately. I'll try it. I do think the number of keybinds needs to be runtime though. Thanks for the naming suggestions! |
Okay this makes sense. My original thought was that this api would be purely for dealing with the builtin dvui keybindings that dvui uses internally - applications that want to make their own custom keybindings for plugins and such would define this functionality on their own. (Such applications can disable builtin dvui keybindings, but not implement new ones - hence an exhaustive enum accomplishes this) If you want to support applications using the builtin dvui keybinding functionality for their own custom keybindings, then using a hashmap here makes sense. It depends on what you would like the purpose of this api to be. |
I tried writing code to use the non-exhaustive enum, but it didn't look very good. So I'm declaring this done for the time being. Please reopen if you run into issues with it. Thank you! |
I first thought about this when considering adding
ctrl+plus
andctrl+minus
as default keybindings to zoom in and out of any dvui application.However, this isn't confined to that use case, should we consider creating a more formal list of dvui control keys that an application can opt out of? Eg something along the lines of
Then a dvui application can still easily use default keybindings:
But the application also has the freedom to disable certain dvui keybindings if they don't want them - or simply choose not to load any dvui keybindings at all.
Thus, if an application prefers for ctrl+c to do something other than copying, they can disable that dvui keybinding. Or if they prefer to use something other than
tab
to navigate between widgets (eg,ctrl+n
) they can load a custom keybinding.Sort of related #112
The text was updated successfully, but these errors were encountered: