feat(behaviors): Step 2 — Normal/Hold-mode slot surface (#16)#29
Merged
Conversation
…tep 2) Add the pure data-model core for the slot-fill binding editor: fill a Tap slot and a Hold slot, derive the behavior instead of naming it first. - src/behaviors/slots.ts: Slots types, a capability-aware DerivationRegistry (resolved by displayName), and deriveBinding / bindingToSlots per the §2 table. isModifierUsage decodes ZMK's implicit-modifier encoding (strips the high-byte mod bits before matching the eight base usages) so multi-modifier holds like LC(LSHFT) survive the binding -> slots -> binding round trip. - Move validateBinding from BehaviorBindingPicker into parameters.ts (its only dependency, validateValue, already lives there) so the tests can assert every derived binding passes the firmware-facing check, and to drop a react-refresh lint warning. - src/behaviors/slots.test.ts: 25 tests covering the §2 table, modifier detection, capability-awareness, binding-lossless round trips, and that every derived binding validates. No firmware/RPC change: the output is still a BehaviorBinding. This is the foundation for the slot UI; the existing chip picker is untouched. Co-Authored-By: Claude <noreply@anthropic.com>
…#16 step 2) Add SlotBindingPicker: the user sets a always-shown **Normal** action (the key grid, with the None/Transparent clear tiles and a collapsible right-side "+ Send with" implicit-modifier column) and an optional **Hold mode** checkbox that reveals a Modifier | Layer picker. The behavior is derived, not named: Normal-only = Key Press, Normal+Hold(mod) = Mod-Tap, Normal+Hold(layer) = Layer-Tap, Hold(layer) alone = Momentary Layer, empty = None. - slots.ts: a lone modifier decomposes to Normal (it's a keycode, `&kp <mod>`), not Hold — so there's no modifier-vs-key heuristic and the round trip stays binding-lossless. Add modifierSetFromUsage / usageFromModifierSet for the multi-select Hold modifier picker (decodes ZMK's implicit-modifier encoding so multi-mod holds like LC(LSHFT) round-trip); drop the now-unused isModifierUsage. - Only Hold mode carries an on/off toggle; Normal is always present (None is a value, not an off state). Labels are Normal / Hold mode — "Tap" is reserved for the derived names (Mod-Tap, Layer-Tap), since a plain &kp isn't short-press-only. - HidUsagePicker gains a `modifiers` config (side / collapsible / label / hint) so the Tap-key implicit-mod column can sit on the right, collapsed, and not be confused with the Hold-mode modifier picker. Default unchanged for other callers. - BehaviorBindingPicker hides the derivation-target chips (kp/mt/lt/mo) + the clear behaviors; the slot surface and residual "Not yet on the slot surface" chips edit one shared local state (no dual-edit sync). The core behaviors (kp/mt/lt/mo/none/trans) are settable now; layer-in-Normal (to/tog/sl) and the single-action tiles come in Steps 3-4. Coverage of every standard ZMK behavior is verified in the design note. Co-Authored-By: Claude <noreply@anthropic.com>
Rewrite §2 around the converged terminology: a always-present **Normal** slot (the key's base action) and an optional **Hold mode** (a different action when held). "Tap" is dropped from the slot labels — it wrongly implies short-press for a plain &kp — and kept only in the derived names (Mod-Tap, Layer-Tap). - §2a: why Normal/Hold mode, and why only Hold mode has an on/off toggle. - §2b: the modifier/layer asymmetry — a lone modifier is a Normal keystroke (`&kp <mod>`), but a lone momentary layer is Hold mode (`&mo`), because modifiers are keycodes and layers are not; a layer in Normal is instead the persistent &to/&tog/&sl. Plus the Windows Sticky-Keys aside. - §4a: coverage audit — every standard ZMK behavior stays settable (core via slots, the rest via the existing param UI / tiles); the model is additive. - Update §4 (modifier-set helpers), Step 2/4, §8 (resolve the lone-modifier question), the status line, and the Japanese §2 mirror. Co-Authored-By: Claude <noreply@anthropic.com>
…old when adding send-with (#16 step 2) UI polish for the slot surface and the keymap keycaps, plus one real bug fix: - HidUsageLabel: show a key's implicit "+ Send with" modifiers as small bordered badges to the left of the cap glyph (e.g. `LC(S)` reads "[Ctrl] S"). Strips the high-byte mod bits before resolving the label so the modified value no longer falls back to a raw `0x…`. - SlotBindingPicker: the "+ Send with" implicit-modifier column is always shown (no longer collapsible) and the NORMAL reverse-lookup label spells out its modifiers (e.g. "L Ctrl + Keyboard S"). - Key / Keymap: tighten keycap padding, top-align header'd keys under the pill, give the hold preview the same weight/opacity as the tap (uniform legibility), and tune the hold/tap row spacing. - behavior-short-names.json: concise header names so the behavior pill no longer truncates ("Mod-Tap", "Lay-Tap", "Sticky", "Caps", "Trans", "To Lyr", ...). Bug fix: `modifiersChanged` in HidUsagePicker was memoized on `[value]` only, capturing a stale `onValueChanged`. With the slot surface, adding a "+ Send with" modifier to a Mod-Tap/Layer-Tap reverted to the pre-hold binding and dropped the hold. Adding `onValueChanged` to the deps keeps the hold (and clears the stale react-hooks warning). Co-Authored-By: Claude <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
zmk-studio-tweaks | b5dad4c | Jun 06 2026, 12:35 PM |
numachang
commented
Jun 6, 2026
numachang
left a comment
numachang
left a comment
Owner
Author
There was a problem hiding this comment.
レビュー: Step 2 — Normal/Hold-mode slot surface
ローカルで検証: 95 tests pass / npm run lint 0 errors / tsc --noEmit clean（lint warning 39 件はすべて既存ファイル由来で、本 PR の新規ファイルには無し）。機能上のブロッカーは見当たりません。approve 寄りです。
良い点
- 純粋モデルの分離 —
slots.tsに派生ロジックを閉じ込め、UI なしで 25 ケースを単体テスト。deriveBindingの出力が全件validateBinding（firmware 相当のゲート）を通ることまで assert しており、「派生 binding が firmware に弾かれない」ことが構造的に保証されている。 - 単一の真実源 — slot 面と残存 chip が同じ
behaviorId/param1/param2を編集し、別の slot state を持たない。設計が警戒していた dual-edit sync を実際に回避できている。 modifiersChangedの stale-closure 修正が正しい —useCallbackdeps を[value]→[value, onValueChanged]に。旧依存だと古いonValueChangedを掴み、Mod-Tap に send-with mod を足すと hold が落ちていた説明と整合。実害のあるバグの良い修正。validateBindingをparameters.tsへ移してテストから参照可能にした分離も妥当。
指摘事項（マージは止めない範囲）
1. 〔Low〕EXPERIMENT: の置き土産コメント — src/keyboard/HidUsageLabel.tsx に // EXPERIMENT: badges at text-xs (...) が残存。削除するか恒久コメントに。
2. 〔Low〕badge が 4 修飾で 4 つ目を黙って落とす — modBadges(...).slice(0, 3)。Ctrl/Shift/Alt/GUI を全部含むキーで GUI バッジが消え、キーキャップが実態を取りこぼす。稀だが「黙って切る」より 4 つ目許容か省略記号が無難。
3. 〔UX / 要検討〕非コア binding でも slot 面が常時表示される — SlotBindingPicker は無条件レンダリングなので、Sticky Key 等を選ぶと上部に「Slots = No matching behavior」＋空の Normal グリッドが出て、その空グリッドのキーを押すと警告なく &kp に上書きされる。Step 3-4 で解消される過渡仕様なのは理解しているが、main に出す前には「非コア時はグリッドを畳む／無効化」か上書き前のヒントが欲しい。
4. 〔Nit〕slots.ts 冒頭コメントの表が旧モデルのまま — ヘッダ表が旧「Tap slot / Hold slot」枠で、(empty) | modifier | &kp <mod> 行を残している。実装の bindingToSlots は「単独修飾は Tap へ」（§2b で収束）なのでコメントと実コードがズレ。新規ファイルのヘッダだけでも Normal/Hold-mode 表に揃えると親切。
5. 〔Minor〕deriveBinding が null のとき writeSlots が無反応 — firmware が当該 behavior を expose していないと onBindingChanged が呼ばれず UI が無反応。設計 §3b の reverse-lookup ラベルに繋ぐ前提のはずで、capability-aware の挙動として現状は許容範囲。
総じて 1・4 は軽微なので今コミットに足しても main 前にまとめても可。3 だけは main に出す前の必須対応として記録を推奨。
… all send-with badges, fix stale doc comment (#16 step 2) - HidUsageLabel: remove the leftover `EXPERIMENT:` comment; show all send-with badges instead of `slice(0, 3)` so a 4-modifier ("hyper") key no longer silently drops its GUI badge. - slots.ts: rewrite the header-comment table to the converged Normal / Hold-mode model (it still described the old Tap/Hold rows incl. the now-wrong "(empty) + modifier → &kp <mod>" — a lone modifier is a Normal keystroke, §2b). - docs(design) §8a: record the before-main blockers raised in review — the slot surface rendering for non-core bindings (empty grid silently overwrites to &kp), and the silent `deriveBinding → null` no-op. Co-Authored-By: Claude <noreply@anthropic.com>
Owner
Author
|
レビューありがとう。対応しました（
ローカル gate: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Step 2 of the key-picker rethink (issue #16): the binding editor gains a
Normal / Hold mode slot surface that derives the behavior instead of
naming it first. Targets the integration branch
feature/key-picker-rethink(not
main) — nothing lands onmainuntil the whole rethink is quality-confirmed.What's in here
src/behaviors/slots.ts+slots.test.ts, 25 tests): acapability-aware
DerivationRegistry(resolved by displayName),deriveBinding/bindingToSlots, andmodifierSetFromUsage/usageFromModifierSet(decode ZMK's implicit-modifier encoding).
validateBindingmoved toparameters.tsso tests assert every derived binding also passes thefirmware-facing check.
SlotBindingPicker.tsx): an always-shown Normal key grid(with the None/Transparent clear tiles and a right-side "+ Send with"
implicit-modifier column) and an optional Hold mode checkbox revealing a
Modifier | Layer picker. Derives Key Press / Mod-Tap / Layer-Tap / Momentary
Layer / None; only the four core behaviors' chips are hidden (single source of
truth, no dual-edit sync).
HidUsageLabel/Key/Keymap): show a key's"+ Send with" implicit modifiers as small bordered badges; tighten keycap
padding, top-align header'd keys, uniform legibility for hold/tap; concise
header short-names so the behavior pill no longer truncates.
modifiersChangedinHidUsagePickercaptured a staleonValueChanged, so adding a "+ Send with" modifier to a Mod-Tap dropped thehold. Fixed the dep array.
docs/design/key-picker-rethink.md) rewritten around theconverged Normal/Hold-mode model, with a coverage audit (§4a) confirming every
standard ZMK behavior stays settable.
Notes
(Step 3), layer-in-Normal
&to/&tog/&sl(Step 4), retire the old tabhierarchy (Step 5).
npm run lint/tscclean, 95 Vitest tests pass.Built on the upstream zmkfirmware/zmk-studio (Apache-2.0); thanks to the ZMK community.
🤖 Generated with Claude Code