zmk-studio icon indicating copy to clipboard operation
zmk-studio copied to clipboard

Show Layer-Tap/Mod-Tap information on keys

Open BafS opened this issue 1 year ago • 19 comments

Show "Layer-Tap" information on the keys, before it was empty, we now see which layer will be enabled on hold and which key will be send on tap.

Instead of the layer logo I was hesitant to add "Hold" to show that this behavior is triggered when it's hold, but it could be done as a future improvement.

Before:

image

After:

image

BafS avatar Apr 10 '25 23:04 BafS

Deploy Preview for zmk-studio ready!

Name Link
Latest commit 4ec09be337f695b51dc20cd879db8c971c5aab56
Latest deploy log https://app.netlify.com/projects/zmk-studio/deploys/692f86971b38270007f52932
Deploy Preview https://deploy-preview-135.preview.zmk.studio
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

netlify[bot] avatar Apr 10 '25 23:04 netlify[bot]

This looks very similar to https://discordapp.com/channels/719497620560543766/722441520741089317/1293320175482966037, which I quite like the look of. If you're able to adjust it to match more closely, that would be great.

Does this work with all hold-taps? If not, it should be extended as such.

nmunnich avatar Apr 11 '25 10:04 nmunnich

Updated the design and made it work with mode-tap too

image

BafS avatar Apr 11 '25 16:04 BafS

Added indicators and some fine tuning

image

BafS avatar Apr 11 '25 18:04 BafS

I agree that it would be nice to see the layer and mod tap info on the keys, but I think we should start out with a simpler visualisation like

image image

This is personal preference though. I feel its simpler to review, and improve over time, when making as few changes as possible.

Also the linting changes you (or your editor) has done, makes it more difficult to review, and should be done in a seperate PR. The actual important changes are confined to 1 or 2 files.

danielsvane avatar Jun 17 '25 13:06 danielsvane

Any updates on this?

alvinometric avatar Aug 15 '25 08:08 alvinometric

I wish sorry, I pinged the devs a few times on Discord, I don't know what to do to get it reviewed again/merged. I lost motivation.

BafS avatar Aug 18 '25 16:08 BafS

Last I checked, Pete is planning on doing more with Studio after we bump Zephyr to 4.1.

nmunnich avatar Aug 18 '25 16:08 nmunnich

Is there still hope to help me review/merge this MR?

BafS avatar Nov 17 '25 09:11 BafS

I believe this PR as-is is seen as too specific for the current state of Studio. See https://discord.com/channels/719497620560543766/719909884769992755/1435008086329917521 for a recent discussion we had - I believe that a generic "display both params of behaviour with two params" needs to come first, then figure out how to make that look good, then do edge cases for things like layer-tap.

I do think this PR could be modified to cover the more general case, but I would understand (yet feel saddened if you feel too demotivated to do said work.

nmunnich avatar Nov 17 '25 15:11 nmunnich

I believe this PR as-is is seen as too specific for the current state of Studio. See https://discord.com/channels/719497620560543766/719909884769992755/1435008086329917521 for a recent discussion we had - I believe that a generic "display both params of behaviour with two params" needs to come first, then figure out how to make that look good, then do edge cases for things like layer-tap.

I do think this PR could be modified to cover the more general case, but I would understand (yet feel saddened if you feel too demotivated to do said work.

So this is basically what @danielsvane suggested above.

If this change would be green lit, I'd be happy to take a stab at it

awkannan avatar Nov 17 '25 17:11 awkannan

I'd love to see improvements here, yes, that align with the metadata approach so we can have a sane display for any possible parameter combination, with enhanced/imporved display for specific pairs of parameter types.

Important note: The calculation of the second parameter type can depend on the given first parameter value, so the calculation for what that second type is can be non-trivial. We do that right now for the behavior parameter picker here, if you need to see that in action: https://github.com/zmkfirmware/zmk-studio/blob/main/src/behaviors/BehaviorParametersPicker.tsx

petejohanson avatar Nov 17 '25 17:11 petejohanson

Ok, thank you guys for the quick answers, I'll try to update this PR one of those days and make it more simple.

BafS avatar Nov 17 '25 20:11 BafS

Still a WIP but it's now closer to the behavior param picker

image

What would be good, I think, is to send the behavior name in the protocol. Currently, we don't have this information, we only have the "display_name" but not the "behavior_name" (see behavior_subsystem.c in the firmware). With this info we could have a design closer to https://nickcoutsos.github.io/keymap-editor/ for instance.

BafS avatar Nov 17 '25 22:11 BafS

Since we pretty much have to deal with the protocol we have now, I think using display_name would be a good solution for now. We can consider using the shorter behavior name later.

Maybe something like this: image

awkannan avatar Nov 17 '25 22:11 awkannan

If we go that route @awkannan I think it would be better to open a separate PR that only change the design. Right now we show this info when we have the cursor "on hover".

I would gladly take care of the implementation, but to avoid back-and-forth it would help to have a specific design/idea/goal in mind before I start.

And thanks for your review! I'll check this soon

BafS avatar Nov 17 '25 23:11 BafS

Agreed - I think that should be a separate PR!

awkannan avatar Nov 17 '25 23:11 awkannan

This would close #147 which would be great 😍

peterjc avatar Nov 18 '25 19:11 peterjc

Since #141 was merged, there are some merge conflicts now. But hopefully it's not too bad to fix up!

awkannan avatar Nov 20 '25 14:11 awkannan

I still need to work on it, but that's how it currently looks like with the rebase from main

image

BafS avatar Dec 03 '25 00:12 BafS