-
Notifications
You must be signed in to change notification settings - Fork 79
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
Feature: tri-state layers #124
base: master
Are you sure you want to change the base?
Conversation
src/layout.rs
Outdated
if (last_layer == self.activation_layers.0 | ||
&& second_to_last_layer == self.activation_layers.1) | ||
|| (last_layer == self.activation_layers.1 | ||
&& second_to_last_layer == self.activation_layers.0) | ||
{ | ||
Some(self.target_layer) | ||
} else { | ||
None | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine, but maybe a match statement would be slightly neater?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the following is a little more compact:
fn apply(&self, layer_0: usize, layer_1: usize) -> Option<usize> {
(self.activation_layers == (layer_0, layer_1)
|| self.activation_layers == (layer_1, layer_0))
.then_some(self.target_layer)
}
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome
src/layout.rs
Outdated
activation_layers, | ||
target_layer, | ||
}) | ||
.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.unwrap()
is fine I think but I believe .expect()
is favoured?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the unwrap
to an expect
. Thank you for the hint.
The current behavior is wanted. At the first version of keyberon, the pressed layers was added to give the current active layer (so plessing l(1) and l(2) will activate l(3)). This behavior was less flexible than the current one, and would not allow "path" to layer. |
Can you give the "equivalent" QMK feature? QMK has lots of strange and interleaved features, and keyberon aims for a condensed list of coherent features. |
Note: I renamed this functionality to The suggested solution is "opt-in" so that the "current behavior" still remains as the default. One can configure such a new tri-layer by calling the let mut layout = Layout::new(&some_layers);
layout.add_tri_layer((1, 2), 3); In QMK, this functionality is described here: https://docs.qmk.fm/#/ref_functions?id=update_tri_layer_statestate-x-y-z |
6dcf69c
to
1834c13
Compare
This PR adds "tri-state layers" that become active if two configured layer modifiers are held at the same time (QMK has something similar).
I previously achieved such a functionality by placing a layer modifier for the third layer on the modifier layers corresponding to the other layer modifiers. In that case, however, the third layer remains active, even when the first hit layer modifier is released.
Example for layer modifiers m1 and m2 leading to layers l1 and l2 if pressed by themselves and a third "tri-state layer" l3:
Intended effect:
press m1 -> l1
press m2 -> l3
release m1 -> l2
release m2 -> default layer
My previous hack-solution gave:
press m1 -> l1
press m2 -> l3
release m1 -> remain on l3
release m2 -> default layer