-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat: default layer setter #2222
base: main
Are you sure you want to change the base?
feat: default layer setter #2222
Conversation
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.
Being able to set the default layer on startup would be wonderful for multi layout configurations.
If I may add an idea to be considered in your follow up PR. Might be worth to consider being able to trigger automatic layout changes on BLE profile change. Not sure how this would be best made configurable (maybe via KConfig?, Could ZMK remember default layer per profile?), but it would be convenient if someone with Windows and macOS system for example, could configure a default layer for each BLE profile and one for USB to automatically trigger a default layer change depending on which profile is activated. |
I acknowledge the potential advantage of the planned behavior for users with dual systems, but after thinking about it a bit more I'm wondering whether this proposed change should be directly integrated into the feature's implementation itself rather than as its separate PR. |
I dont mind either way, the idea of making a small PR was to start rolling it and see if it had missed a small edge case or whatever. Also making two different PRs with the respective self-contained logic seemed better review-wise. I already have the code (or at least, its first revision) written & slightly tested it (with to instead of setting default layer at ZMK level). So, if collabs think it is better to keep everything together just let me know and i will push it on this branch too |
Sorry, I have been quite busy lately. Personally, I would recommend adding everything related to this feature in one PR so when it's merged it adds the full feature. I tested your code from your personal repo and I would recommend to incorporate it into this PR for full review/testing of the complete feature. |
Aight, will do. Again, my main idea was go test if default layer setting really worked well before relying on it for the behavior, for easier debug when (if) edge cases poped up. Will add it to make the process faster and keep it together. Glad to hear the current implementation (layer toggle instead of default layer, which is worse) is working fine for you |
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.
One major concern regarding the usage of the endpoints API. Thanks for the PR!
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.
Two other docs pieces need updating in addition to below comments:
- https://zmk.dev/docs/behaviors#layer-navigation-behaviors table
- The default layer remark in https://zmk.dev/docs/config/keymap#devicetree
#1500 seems like it could be relevant here and hasn't been mentioned yet, just dropping a link. |
@petejohanson I'm interested in having this feature get merged. Would you have time in the near future to re-review this PR? |
I plan to do a PR to persistently change the default layer, for multi-layout uses (eg win/mac or qwerty/dvorak)
However, while activating the layer or to'ing into it is similar, it wouldn't mimic the behavior of default layer. Thus, the follow-up PR needs an API of this kind.
Notes: