Conversation
PR #541 added read-only "View …" sub-modes to the seven directional setup actions. This turns those View sub-modes into one-key dual-press controls: while the key shows the live telemetry value, a short press fires one direction and a long press fires the opposite — replacing two physical keys (separate Increase + Decrease) with one that also displays the value. - deck-core: new DualPressTracker (per-context tap/long-press timing), dualPressThresholdMs (200-2000 ms, default 500) and dualPressDirections ("tap-increases" | "tap-decreases") global settings, with getDualPressThresholdMs() / getDualPressDirections() readers. - iracing-actions: each setup action gains a per-action dualPressEnabled opt-in (default on); View sub-modes record key-down on onKeyDown and dispatch the resolved direction on onKeyUp via the existing global key bindings. setup-view.ts maps each View id to its adjustment mode; setup-chassis adds dispatch-only weight-jacker key bindings. - pi-components: dual-press opt-in partial, global threshold + directions rows in Common Settings, and a shared ird-supporting-text CSS class for help text (replaces inline styles). - Docs/website/skill updated; tap direction and threshold are documented as plugin-wide Global Common Settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughImplements opt-in dual-press across directional setup actions: core tracker + global settings, PI controls and CSS, per-action opt-in and onKeyUp dispatch that maps short/long presses to increase/decrease, tests, and documentation updates. ChangesDual-press mode for setup actions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/website/src/content/docs/docs/actions/car-setup/setup-chassis.md (1)
14-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winResolve contradictory View-mode behavior wording.
This section documents interactive dual-press View behavior, but the later Modes text still says View modes are read-only. Please align both sections so users don’t get conflicting guidance.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/website/src/content/docs/docs/actions/car-setup/setup-chassis.md` around lines 14 - 35, The docs currently contradict: the "Dual-press control" section explains interactive dual-press behavior for View modes (including the "Enable dual-press" Property Inspector setting and the global "Dual-Press → Directions" and "Long-press threshold (ms)") but the later "Modes" text calls View modes read-only; update the "Modes" text to either remove the blanket "read-only" claim or add an explicit exception clarifying that View modes are read-only only when "Enable dual-press" is off and otherwise dispatch adjustment bindings (e.g., Diff Preload → Diff Preload +/−, Weight Jacker bindings in Global Settings → Setup Chassis); ensure the wording references "Enable dual-press", "Global Common Settings → Dual-Press → Directions" and "Long-press threshold (ms)" so readers know the toggle and global settings control interactive behavior.
🧹 Nitpick comments (2)
packages/iracing-actions/src/actions/setup-hybrid/setup-hybrid.test.ts (1)
68-76: ⚡ Quick winAdd explicit tests for View dual-press key-up dispatch.
The mock now supports dual-press, but this suite still doesn’t assert the new
onKeyUpbehavior (dualPressEnabled=truedispatch,falseno dispatch). Adding those cases would protect the new path from regressions.Suggested test skeleton
+it("dispatches mapped tap/long direction on keyUp for View mode", async () => { + const action = new SetupHybrid(); + const tracker = (action as any).dualPress; + tracker.computeOutcome.mockReturnValue("increase"); + + await action.onKeyUp(fakeEvent("action-1", { + setting: "view-mguk-deploy-mode", + dualPressEnabled: true, + }) as any); + + expect(mockTapBinding).toHaveBeenCalledWith("setupHybridMgukDeployModeIncrease"); +}); + +it("does not dispatch on keyUp when dual-press is disabled", async () => { + const action = new SetupHybrid(); + await action.onKeyUp(fakeEvent("action-1", { + setting: "view-mguk-deploy-mode", + dualPressEnabled: false, + }) as any); + expect(mockTapBinding).not.toHaveBeenCalled(); +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/iracing-actions/src/actions/setup-hybrid/setup-hybrid.test.ts` around lines 68 - 76, Add explicit tests in setup-hybrid.test.ts that exercise the onKeyUp dual-press dispatch path: use the existing MockDualPressTracker (and helper mocks getDualPressThresholdMs/getDualPressDirections) to simulate a computed dual-press outcome and assert that when dualPressEnabled=true the dispatcher/action for dual-press is called on key-up, and when dualPressEnabled=false no dual-press dispatch occurs; specifically, create two cases that stub MockDualPressTracker.computeOutcome to return a dual-press result and verify dispatch behavior on the component/module's onKeyUp handler, and another case where dualPressEnabled is false to verify no dispatch.packages/iracing-actions/src/actions/setup-fuel/setup-fuel.test.ts (1)
60-67: ⚡ Quick winAdd action-level tests for dual-press View behavior on key-up.
This suite mocks dual-press primitives but doesn’t currently assert the new
onKeyUpdispatch path (enabled/disabled modes). Adding those cases will lock in the intended behavior.Suggested test skeleton
+it("dispatches View adjustment on keyUp when dual-press is enabled", async () => { + const action = new SetupFuel(); + const tracker = (action as any).dualPress; + tracker.computeOutcome.mockReturnValue("increase"); + + await action.onKeyUp(fakeEvent("action-1", { + setting: "view-fuel-mixture", + dualPressEnabled: true, + }) as any); + + expect(mockTapBinding).toHaveBeenCalledWith("setupFuelFuelMixtureIncrease"); +}); + +it("does not dispatch on keyUp when dual-press is disabled", async () => { + const action = new SetupFuel(); + await action.onKeyUp(fakeEvent("action-1", { + setting: "view-fuel-mixture", + dualPressEnabled: false, + }) as any); + expect(mockTapBinding).not.toHaveBeenCalled(); +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/iracing-actions/src/actions/setup-fuel/setup-fuel.test.ts` around lines 60 - 67, Add tests in setup-fuel.test.ts that assert the action dispatch path on key-up depending on dual-press mode: when getDualPressDirections is mocked to "tap-increases" simulate a keyUp through the action's handler (use the existing MockDualPressTracker and set computeOutcome to return the dual-press outcome you expect) and assert the onKeyUp dispatch branch was called (spy on the action dispatch/mock and verify payload); then add a complementary test where getDualPressDirections is mocked to a disabled value and verify the onKeyUp dispatch branch is not invoked. Use the existing MockDualPressTracker methods (recordKeyDown, computeOutcome, clear, hasPending) to drive state and ensure you reset/restore mocks between tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/deck-core/src/dual-press.test.ts`:
- Around line 186-191: The test is writing a recognized value so it never
exercises the fallback; change the stored passthrough value written via
updateGlobalSettings to an invalid string (e.g. "invalid-direction" or
"tap-unknown") after calling initGlobalSettings(createMockAdapter(),
createMockLogger()) so that getDualPressDirections() must hit the fallback path
and then assert the expected fallback result (the default/normalized value your
code returns). Use the same helpers mentioned (initGlobalSettings,
updateGlobalSettings, getDualPressDirections) to locate and modify the test.
In `@packages/iracing-actions/src/actions/setup-traction/setup-traction.test.ts`:
- Around line 66-73: The test adds DualPressTracker mocks but doesn't assert
runtime behavior; add tests that use the MockDualPressTracker (DualPressTracker,
recordKeyDown, computeOutcome, clear, hasPending) and
getDualPressThresholdMs/getDualPressDirections to simulate short and long key
presses and verify the View's onKeyUp dispatches the expected actions for
"tap-increases" vs long-press outcomes, assert computeOutcome was called with
the right timestamps and that clear/hasPending are used appropriately, and add a
test where dualPressEnabled=false to confirm the code falls back to single-press
behavior (no dual-press outcome dispatched).
In `@packages/website/src/content/docs/docs/actions/car-setup/setup-engine.md`:
- Around line 14-29: The page currently contradicts itself by describing View
entries as both read-only and interactive with dual-press; update the "Modes"
section language to match the "View …" and "Dual-press control" sections by
stating that View modes normally show a live readout but, when the
property-inspector setting "Enable dual-press" is on (default on) they also
accept short/long presses that map to the corresponding adjustment bindings;
also add a brief note in the Modes text referencing the global Dual-Press
direction and Long-press threshold settings so users know where to change tap
behavior.
---
Outside diff comments:
In `@packages/website/src/content/docs/docs/actions/car-setup/setup-chassis.md`:
- Around line 14-35: The docs currently contradict: the "Dual-press control"
section explains interactive dual-press behavior for View modes (including the
"Enable dual-press" Property Inspector setting and the global "Dual-Press →
Directions" and "Long-press threshold (ms)") but the later "Modes" text calls
View modes read-only; update the "Modes" text to either remove the blanket
"read-only" claim or add an explicit exception clarifying that View modes are
read-only only when "Enable dual-press" is off and otherwise dispatch adjustment
bindings (e.g., Diff Preload → Diff Preload +/−, Weight Jacker bindings in
Global Settings → Setup Chassis); ensure the wording references "Enable
dual-press", "Global Common Settings → Dual-Press → Directions" and "Long-press
threshold (ms)" so readers know the toggle and global settings control
interactive behavior.
---
Nitpick comments:
In `@packages/iracing-actions/src/actions/setup-fuel/setup-fuel.test.ts`:
- Around line 60-67: Add tests in setup-fuel.test.ts that assert the action
dispatch path on key-up depending on dual-press mode: when
getDualPressDirections is mocked to "tap-increases" simulate a keyUp through the
action's handler (use the existing MockDualPressTracker and set computeOutcome
to return the dual-press outcome you expect) and assert the onKeyUp dispatch
branch was called (spy on the action dispatch/mock and verify payload); then add
a complementary test where getDualPressDirections is mocked to a disabled value
and verify the onKeyUp dispatch branch is not invoked. Use the existing
MockDualPressTracker methods (recordKeyDown, computeOutcome, clear, hasPending)
to drive state and ensure you reset/restore mocks between tests.
In `@packages/iracing-actions/src/actions/setup-hybrid/setup-hybrid.test.ts`:
- Around line 68-76: Add explicit tests in setup-hybrid.test.ts that exercise
the onKeyUp dual-press dispatch path: use the existing MockDualPressTracker (and
helper mocks getDualPressThresholdMs/getDualPressDirections) to simulate a
computed dual-press outcome and assert that when dualPressEnabled=true the
dispatcher/action for dual-press is called on key-up, and when
dualPressEnabled=false no dual-press dispatch occurs; specifically, create two
cases that stub MockDualPressTracker.computeOutcome to return a dual-press
result and verify dispatch behavior on the component/module's onKeyUp handler,
and another case where dualPressEnabled is false to verify no dispatch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dca80caf-e045-4e80-bfd9-934a930a8ec7
📒 Files selected for processing (42)
.claude/rules/pi-templates.md.claude/skills/iracedeck-actions/SKILL.mdpackages/deck-core/src/dual-press.test.tspackages/deck-core/src/dual-press.tspackages/deck-core/src/global-settings.test.tspackages/deck-core/src/global-settings.tspackages/deck-core/src/index.tspackages/deck-core/src/simhub-service.test.tspackages/iracing-actions/src/actions/data/key-bindings.jsonpackages/iracing-actions/src/actions/setup-aero/setup-aero.ejspackages/iracing-actions/src/actions/setup-aero/setup-aero.test.tspackages/iracing-actions/src/actions/setup-aero/setup-aero.tspackages/iracing-actions/src/actions/setup-brakes/setup-brakes.ejspackages/iracing-actions/src/actions/setup-brakes/setup-brakes.test.tspackages/iracing-actions/src/actions/setup-brakes/setup-brakes.tspackages/iracing-actions/src/actions/setup-chassis/setup-chassis.ejspackages/iracing-actions/src/actions/setup-chassis/setup-chassis.test.tspackages/iracing-actions/src/actions/setup-chassis/setup-chassis.tspackages/iracing-actions/src/actions/setup-engine/setup-engine.ejspackages/iracing-actions/src/actions/setup-engine/setup-engine.test.tspackages/iracing-actions/src/actions/setup-engine/setup-engine.tspackages/iracing-actions/src/actions/setup-fuel/setup-fuel.ejspackages/iracing-actions/src/actions/setup-fuel/setup-fuel.test.tspackages/iracing-actions/src/actions/setup-fuel/setup-fuel.tspackages/iracing-actions/src/actions/setup-hybrid/setup-hybrid.ejspackages/iracing-actions/src/actions/setup-hybrid/setup-hybrid.test.tspackages/iracing-actions/src/actions/setup-hybrid/setup-hybrid.tspackages/iracing-actions/src/actions/setup-traction/setup-traction.ejspackages/iracing-actions/src/actions/setup-traction/setup-traction.test.tspackages/iracing-actions/src/actions/setup-traction/setup-traction.tspackages/iracing-actions/src/shared/setup-view.tspackages/pi-components/partials/dual-press-overrides.ejspackages/pi-components/partials/global-common-settings.ejspackages/pi-components/partials/global-flag-flash.ejspackages/pi-components/partials/head-common.ejspackages/website/src/content/docs/docs/actions/car-setup/setup-aero.mdpackages/website/src/content/docs/docs/actions/car-setup/setup-brakes.mdpackages/website/src/content/docs/docs/actions/car-setup/setup-chassis.mdpackages/website/src/content/docs/docs/actions/car-setup/setup-engine.mdpackages/website/src/content/docs/docs/actions/car-setup/setup-fuel.mdpackages/website/src/content/docs/docs/actions/car-setup/setup-hybrid.mdpackages/website/src/content/docs/docs/actions/car-setup/setup-traction.md
- dual-press.test.ts: the "unrecognised value" case now genuinely exercises the getDualPressDirections fallback by stubbing getGlobalSettings (the schema enum rejects invalid values, so they can't reach the reader via the normal cache path). Added a sanity assertion that the spy is intercepting. - Added full dual-press behavior tests to the remaining six setup actions (traction, fuel, engine, chassis, hybrid, aero) — previously only setup-brakes had them. setup-hybrid's variant asserts the merged hold/release path still routes non-View key-ups to releaseBinding. - setup-engine.md / setup-chassis.md: the "Modes" section no longer calls View modes read-only — reconciled with the dual-press "View sub-modes" section.
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.
Related Issue
Fixes #540
What changed?
PR #541 added read-only "View …" sub-modes to the seven directional setup actions. This PR turns those View sub-modes into one-key dual-press controls: while the key shows the live telemetry value, a short press fires one direction and a long press fires the opposite — replacing two physical keys (separate Increase + Decrease) with one that also displays the value.
@iracedeck/deck-core— newDualPressTracker(per-context tap/long-press timing keyed by action context id). Two new global settings:dualPressThresholdMs(200–2000 ms, default 500) anddualPressDirections(tap-increases|tap-decreases, defaulttap-increases), withgetDualPressThresholdMs()/getDualPressDirections()readers. Both are plugin-wide so the tap-vs-long-press rule stays consistent across every setup action.@iracedeck/iracing-actions— each of the 7 setup actions gains a per-actiondualPressEnabledopt-in (default on). When enabled, a View sub-mode records key-down inonKeyDownand dispatches the resolved direction inonKeyUpvia the existing global key bindings.setup-view.tsmaps each View id to its underlying adjustment mode (getAdjustmentModeForView);setup-chassisadds dispatch-only weight-jacker key bindings (no matching adjustment mode existed). Directional adjustment, toggle, and encoder behaviour are untouched.@iracedeck/pi-components— newdual-press-overridespartial (the per-action enable checkbox), global threshold + directions rows in the Common Settings accordion, and a sharedird-supporting-textCSS class for help text (replaces inline styles acrossglobal-common-settings.ejsandglobal-flag-flash.ejs)..claude/rules/pi-templates.mddocuments theird-supporting-textclass.How to test
pnpm install && pnpm build && pnpm test && pnpm lint:fix && pnpm format:fix— all green (3944 tests).+/-global key bindings, drop a "Setup Brakes" key, pickview-brake-bias. Verify the live value renders.Checklist
Summary by CodeRabbit
New Features
Documentation
Tests