feat(behaviors): Step 3a — keycap behavior tiles in the Normal grid (#16)#30
Conversation
) Tile-ify the single-action behaviors and fold them into the Normal-slot key grid's tab strip, building toward the converged tile surface incrementally. - tiles.ts: tileBindingsFor explodes a behavior into one keycap tile per finished (param1,param2) binding; null when a param needs a rich picker. - Mouse + a consolidated System tab (Bluetooth/Output/External Power/System) live in the Normal grid's tab strip between International and Other; picking a behavior tile sets the whole binding (HidUsagePicker extraTabs/onSelectBinding). - Collapse the two basic-tier tabs into one host-adaptive "Basic" tab (ansi->ANSI, iso->ISO+NUHS/NUBS, jis->JIS, ko->KO); ISO keys appear once that layout is selected and stay searchable. - "+ Send with" moves from a side column to an inline row below the grid (Basic / Function+Nav only); drop the "KEY" header and host the search on the NORMAL row; the filter now spans the behavior tiles too. - Slot label shows the devicetree code (formatBinding: &mt LSHFT RET, &mo 1, &reset, ...) in a boxed chip, host-layout-aware (JIS Shift+2 = "). - Keycode-picker DRY cleanup: tab-name constants; promote consumer media keys to Apps/Media/Special; group the Other combobox into labelled sections (other-sections.ts). Tests: tiles / formatBinding / other-sections (121 pass). Snapshot files left out. Co-Authored-By: Claude <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
zmk-studio-tweaks | 2076357 | Commit Preview URL Branch Preview URL |
Jun 07 2026, 02:52 AM |
numachang
left a comment
numachang
left a comment
There was a problem hiding this comment.
レビュー — Step 3a: Normal グリッドへの behavior タイル統合
完成度が高く、そのままマージしてよい品質です。設計ドキュメント・コミットメッセージ・型・テストが揃っており、純ロジック（tiles.ts / other-sections.ts / formatBinding）を React から切り離してユニットテスト化している点が特に良いです。base も feature/key-picker-rethink で正しく、フォークの最小差分方針に沿っています。
ローカル検証
npx vitest run（新規3ファイル）→ 26 passnpx tsc --noEmit→ 0 errornpx eslint（変更6ファイル）→ 0 error / 既存 warning 2件のみChevronDownは Other コンボボックスで継続使用（デッドインポートなし）、collapsible/modsOpen/activeModSummary等の旧モディファイア機構の残骸なし
良い点
- host 連動 Basic タブ統合で、ANSI ユーザーに ISO 形状(NUHS/NUBS)が出ていた挙動を解消しタブ数も削減。
enumerateParamがhidUsage/layerIdでnullを返し behavior 全体を非タイル化する分岐がテストで網羅。formatBindingが既存シリアライザを再利用し constant 名を優先。&bt BT_SEL 2/&bt BT_CLRまで検証済み。- 検索が behavior タイルも横断（
reset/bt/mouse）し、以前の「検索がキーコード専用」の穴を解消。
結論: Approve 相当。以下の指摘はすべて Low / 任意対応でブロッカーなし。§8a ブロッカー（非コア binding の空スロット面・黙った &kp 上書き）も実質解消されています。本文どおり main へ統合する前に実機で触り心地を最終確認すれば十分です。
（インラインに Low の指摘を4点付けています）
| if (sets.length === 0) return ref; | ||
|
|
||
| const set = | ||
| sets.find((s) => validateValue(layerIds, binding.param1, s.param1)) ?? sets[0]; |
There was a problem hiding this comment.
[Low / nit] フォールバック ?? sets[0]：param1 がどの set にもマッチしないとき先頭 set を使うため、param2 を持つ set が選ばれると実体と異なる余分なパラメータを表示し得ます。タイル/コア由来の binding は必ず有効なので到達は稀ですが、「マッチしないなら ref だけ返す」方が安全側です。
| label={tile.label} | ||
| > | ||
| <Button | ||
| aria-pressed={active} |
There was a problem hiding this comment.
[Low / a11y] タイル選択のセマンティクスが2箇所で不一致です。ここは role="group" + aria-pressed（トグル意味）ですが、BehaviorBindingPicker の残置領域は radiogroup + role="radio"。「現在の binding を1つ選ぶ」単一選択なので、両方を radio 系に揃える方が支援技術に正確に伝わります（aria-pressed は再クリックで解除できる含みがあるが実際は解除不可）。
| // so it only fires when the binding actually changes — otherwise it would snap | ||
| // the tab back to Mouse/System the moment the user clicks a keycode tab to | ||
| // pick a Normal key. | ||
| useEffect(() => { |
There was a problem hiding this comment.
[情報 / 変更不要] この activeBinding ベースのタブジャンプと、上の value ベースの setActiveTab(null) が同一 render で発火し、最終状態は「後に宣言されたこちらが勝つ」前提で正しく動きます。コメントで意図は説明済みですが、宣言順依存でやや脆い構造である点だけ共有まで。
| setActiveTier(currentClass.tier); | ||
| setActiveGroup(currentClass.group); | ||
| if (currentGroup) setActiveGroup(currentGroup); | ||
| }, [behaviorId]); |
There was a problem hiding this comment.
[Low / nit] exhaustive-deps warning が残ります（currentGroup 欠落）。behaviorId 変化時のみ追従させる意図だと思いますが、他箇所と同様 // eslint-disable-next-line react-hooks/exhaustive-deps を付けると warning が消えて意図も明示できます。329 行目の effect も同様。
- formatBinding: when param1 matches no metadata set, emit just the behavior reference rather than borrowing the first set's params (avoids showing a param2 that doesn't apply). [review #1] - Behavior tiles are single-select, so make them `role="radio"` inside a `radiogroup` (consistent with the residual behavior chips) instead of `aria-pressed` toggles; native button + `title` keeps the visible label and drops the now-redundant tooltip. Search-result tiles get their own radiogroup. [review #2] - Annotate the two intentional exhaustive-deps effects with `eslint-disable-next-line` so the warnings clear and the intent is explicit. [review #4] Review #3 (declaration-order of the two tab-select effects) was info-only / no change requested; left as-is. Co-Authored-By: Claude <noreply@anthropic.com>
|
レビューありがとうございます。Low 指摘4件を
ローカル gate: §8a の実機タッチ最終確認のうえ、 |
1 participant
Step 3a — keycap behavior tiles in the Normal grid (issue #16)
Part of the staged key-picker rethink. Base:
feature/key-picker-rethink(theintegration branch — not
main, not upstream). Step 3 was split into 3a/3b; this is 3a.Plan pivot: rather than parking migrated behaviors in throwaway scaffolding until
Step 5, each step now moves its behaviors toward their final place, so the converged
tile surface is visible (and critiquable) at every step.
What's in 3a
tiles.ts—tileBindingsForexplodes a behavior into one keycap tile perfinished
(param1, param2)binding (enum/range exploded);nullwhen a parameterneeds a rich picker (HID key / layer), which keeps the chip + parameter-picker flow.
live in the Normal grid's tab strip (between International and Other). Picking a
behavior tile sets the whole binding; picking a keycode sets the Normal key.
(
ansi→ANSI,iso→ISO+NUHS/NUBS,jis→JIS,ko→KO). ISO-only keys appear once thatlayout is selected and stay reachable via search. One fewer tab, correct ANSI for US.
Basic / Function+Nav). The redundant "KEY" header is dropped and the search moves onto
the NORMAL row — and now spans the behavior tiles too (
reset,bt,mouse, …).&mt LSHFT RET,&mo 1,&reset,&bt BT_SEL 0) in a boxed chip, host-layout-aware (JIS Shift+2 reads", not@).keys promoted to Apps/Media/Special; the Other combobox grouped into labelled
sections (
other-sections.ts).§8a before-main blocker
The "non-core binding shows a blank slot surface and silently overwrites to
&kp" blockeris largely resolved: the slot label now names the behavior + shows its code, and the
tile is highlighted. Confirm the feel before the integration branch merges to
main.Testing / gates
Intermediate PRs get no CI (
ci.ymlisbranches:[main]), so gated locally:npx tsc --noEmit— 0 errorsnpx eslint src— 0 errors (only pre-existing warnings)npx vitest run— 121 pass (new:tiles,formatBinding,other-sections)The two
src/__snapshots__/*.snapfiles were modified before this work began and areintentionally left out of the commit.
🤖 Generated with Claude Code