zmk icon indicating copy to clipboard operation
zmk copied to clipboard

feat: default layer setter

Open elpekenin opened this issue 11 months ago • 7 comments

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:

  • New event for default layer changes?
  • I dont quite like the function's name and log texts, ideas are welcome.

elpekenin avatar Mar 23 '24 11:03 elpekenin

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.

huber-th avatar Mar 23 '24 15:03 huber-th

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.

huber-th avatar Mar 25 '24 05:03 huber-th

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

elpekenin avatar Mar 25 '24 08:03 elpekenin

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.

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.

huber-th avatar Apr 04 '24 06:04 huber-th

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

elpekenin avatar Apr 04 '24 06:04 elpekenin

#1500 seems like it could be relevant here and hasn't been mentioned yet, just dropping a link.

Nick-Munnich avatar May 10 '24 22:05 Nick-Munnich

@petejohanson I'm interested in having this feature get merged. Would you have time in the near future to re-review this PR?

TrangPham avatar Oct 04 '24 18:10 TrangPham