Repository navigation
Keep an unlinked Logitech X/Y slot unlinked after a refresh #1154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,11 +77,12 @@ function DpiStageEditor({ | |
| const countCap = isLogitech ? (limits?.maxStages ?? 1) : stageEditor?.maxStages ?? 1; | ||
| const rowCap = mode === "generic" ? MAX_EDITOR_ROWS : countCap; | ||
|
|
||
| const initRows = (prev?: StageRow[]): StageRow[] => { | ||
| const initRows = (): StageRow[] => { | ||
| if (mode === "logitech" && snapshot.dpiSlotPlan) { | ||
| const stages = snapshot.dpiSlotPlan.stages.slice(0, rowCap); | ||
| const unlinked = control.unlinkedDpiSlots(); | ||
| const last = stages[stages.length - 1]?.x ?? status?.dpi ?? 800; | ||
| const rows = stages.map((stage, index) => ({ enabled: true, value: String(stage.x), valueY: String(stage.y ?? stage.x), lod: stage.lod, linked: (stage.y ?? stage.x) === stage.x && prev?.[index]?.linked !== false })); | ||
| const rows = stages.map((stage, index) => ({ enabled: true, value: String(stage.x), valueY: String(stage.y ?? stage.x), lod: stage.lod, linked: (stage.y ?? stage.x) === stage.x && !unlinked.includes(index) })); | ||
|
Comment on lines
+80
to
+85
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: set -u
printf '%s\n' '--- DpiCard references and blocks ---'
rg -n -F -- 'unlinkedDpiSlots' 'setDpiSlotUnlinked' 'applyDpiSlotEditor' 'initRows' 'enabled' src/features/devices/cards/DpiCard.tsx src/device/controller.ts || true
printf '%s\n' '--- DpiCard relevant source ---'
sed -n '60,180p' src/features/devices/cards/DpiCard.tsx
sed -n '180,340p' src/features/devices/cards/DpiCard.tsx
sed -n '430,500p' src/features/devices/cards/DpiCard.tsx
printf '%s\n' '--- controller persistence and editor source ---'
sed -n '4560,4675p' src/device/controller.ts
sed -n '4680,4720p' src/device/controller.ts
printf '%s\n' '--- diff for the reviewed change ---'
git diff 5196b90b1d2ba135068293a1892a8b2e8736dd11 94ed0615bc3e94c4741a7b4322162fa0eea43562 -- src/features/devices/cards/DpiCard.tsx src/device/controller.tsRepository: OpenMouse-Project/openmouse Length of output: 26649 Remap unlink preferences when Logitech stages are compacted. When an earlier Logitech stage is disabled, the enabled stages are compacted into new indices. Remap each saved preference to the surviving row’s new index during the Logitech commit. Suggested fix if (mode === "logitech") {
+ const enabledIndices = next
+ .map((row, index) => row.enabled ? index : -1)
+ .filter((index) => index >= 0);
+ const unlinked = control.unlinkedDpiSlots();
+ unlinked.forEach((index) => control.setDpiSlotUnlinked(index, false));
+ unlinked
+ .map((index) => enabledIndices.indexOf(index))
+ .filter((index) => index >= 0)
+ .forEach((index) => control.setDpiSlotUnlinked(index, true));
// Lift-off lives in the slot plan itself now (edited from the dedicated🤖 Prompt for AI Agents |
||
| while (rows.length < rowCap) rows.push({ enabled: false, value: String(last), valueY: String(last), lod: DEFAULT_LOD, linked: true }); | ||
| return rows; | ||
| } | ||
|
|
@@ -160,7 +161,7 @@ function DpiStageEditor({ | |
| useEffect(() => { | ||
| if (sourceSig === lastSigRef.current) return; | ||
| lastSigRef.current = sourceSig; | ||
| if (Date.now() - touchedRef.current > 1500 || (mode === "logitech" && !timerRef.current)) setRows(initRows(rowsRef.current)); | ||
| if (Date.now() - touchedRef.current > 1500 || (mode === "logitech" && !timerRef.current)) setRows(initRows()); | ||
| // initRows is recreated each render on purpose so it always reads fresh snapshot data. | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [sourceSig]); | ||
|
Comment on lines
161
to
167
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '340,385p' src/features/devices/OverviewPage.tsx
sed -n '145,175p' src/features/devices/cards/DpiCard.tsx
rg -n 'openOnboardProfile|editedProfile|activeDevice|selectDevice|setActiveDevice|DpiCard|DpiStageEditor' src/features/devices/OverviewPage.tsx src/features/devices/cards/DpiCard.tsx src/device/controller.tsRepository: OpenMouse-Project/openmouse Length of output: 11638 🏁 Script executed: printf '%s\n' '--- DpiStageEditor state and refresh ---'
sed -n '1,190p' src/features/devices/cards/DpiCard.tsx
printf '%s\n' '--- DpiCard caller ---'
sed -n '590,720p' src/features/devices/cards/DpiCard.tsx
printf '%s\n' '--- Overview caller ---'
sed -n '350,375p' src/features/devices/OverviewPage.tsx
printf '%s\n' '--- device transition lifecycle ---'
sed -n '3300,3470p' src/device/controller.ts
printf '%s\n' '--- profile transition lifecycle ---'
sed -n '4400,4520p' src/device/controller.ts
sed -n '4660,4810p' src/device/controller.ts
printf '%s\n' '--- profile selection/reset callers ---'
sed -n '5100,5210p' src/device/controller.ts
sed -n '5220,5300p' src/device/controller.tsRepository: OpenMouse-Project/openmouse Length of output: 42372 🏁 Script executed: rg -n -F -- 'export type ControlSnapshot' src/device
rg -n -F -- 'ControlSnapshot' src/device/controller.ts src/device/types.ts | head -40
rg -n -F -- 'activeDevice' src/device/types.ts src/device/controller.ts | head -60
sed -n '430,610p' src/device/controller.tsRepository: OpenMouse-Project/openmouse Length of output: 10590 Remount When a Logitech device or onboard profile changes without changing the stage X/Y values, Suggested fix export function DpiCard({ snapshot }: { snapshot: ControlSnapshot }): ReactNode {
// A table with one value in every stage is what Single saved, so reopen in that view.
const [editorView, setEditorView] = useState<"stage" | "single">(() => (
singleFillsStageTable(snapshot) && isUniform(snapshot.deviceStatus?.dpiStages) ? "single" : "stage"
));
+ const activeDevice = control.getActiveDevice();
+ const editorIdentity =
+ `${activeDevice?.vendorId ?? 0}:${activeDevice?.productId ?? 0}:${activeDevice?.productName || "unknown"}#${snapshot.editedProfile ?? "live"}`;
const status = snapshot.status;
const deviceStatus = snapshot.deviceStatus;
const locale = snapshot.preferences.locale;
@@
<DpiStageEditor
+ key={editorIdentity}
snapshot={snapshot}
editorView={stageCapable && singleDpiAvailable ? editorView : stageCapable ? "stage" : undefined}
/>🤖 Prompt for AI Agents |
||
|
|
@@ -190,6 +191,10 @@ function DpiStageEditor({ | |
| // side panel), so a DPI-value commit must not clobber it with the local | ||
| // row copy. Enabled rows map 1:1 to plan stages in order. | ||
| const planLods = snapshot.dpiSlotPlan?.stages.map((stage) => stage.lod) ?? []; | ||
| // Disabled rows drop out of the plan, so saved unlink rows move to their compacted index. | ||
| control.replaceDpiUnlinkedSlots( | ||
| next.filter((row) => row.enabled).flatMap((row, position) => (row.linked === false ? [position] : [])), | ||
| ); | ||
| control.applyDpiSlotEditor( | ||
| next.map((row, index) => { | ||
| const position = next.slice(0, index).filter((entry) => entry.enabled).length; | ||
|
|
@@ -314,11 +319,9 @@ function DpiStageEditor({ | |
| }; | ||
|
|
||
| const toggleLink = (index: number): void => { | ||
| const next = rows.map((row, i) => { | ||
| if (i !== index) return row; | ||
| const linked = row.linked === false; | ||
| return { ...row, linked, valueY: linked ? row.value : row.valueY }; | ||
| }); | ||
| const linked = rows[index]?.linked === false; | ||
| control.setDpiSlotUnlinked(index, !linked); | ||
| const next = rows.map((row, i) => (i === index ? { ...row, linked, valueY: linked ? row.value : row.valueY } : row)); | ||
| setRows(next); | ||
| rowsRef.current = next; | ||
| markEdited(next); | ||
|
|
@@ -705,6 +708,7 @@ export function DpiCard({ snapshot }: { snapshot: ControlSnapshot }): ReactNode | |
| </div> | ||
|
|
||
| <DpiStageEditor | ||
| key={control.dpiUnlinkKey() ?? "dpi"} | ||
| snapshot={snapshot} | ||
| editorView={stageCapable && singleDpiAvailable ? editorView : stageCapable ? "stage" : undefined} | ||
| /> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: OpenMouse-Project/openmouse
Length of output: 7281
🏁 Script executed:
Repository: OpenMouse-Project/openmouse
Length of output: 9119
Guard malformed DPI unlink storage on read and toggle.
The normal writer stores an object whose active entry is an array, so it does not create these malformed shapes from valid state. However, valid JSON such as
nullor an object with a non-array active entry can remain inlocalStorageafter manual or external storage corruption. The Logitech editor can then throw duringinitRows(). The proposedunlinkedDpiSlots()guard prevents the render failure, butsetDpiSlotUnlinked()also needs to validate the active entry before constructing theSet.Proposed fix
function unlinkedStore(): Record<string, number[]> { try { - return JSON.parse(localStorage.getItem(DPI_UNLINKED_STORAGE_KEY) ?? "{}") as Record<string, number[]>; + const parsed: unknown = JSON.parse(localStorage.getItem(DPI_UNLINKED_STORAGE_KEY) ?? "{}"); + return parsed && typeof parsed === "object" && !Array.isArray(parsed) ? parsed as Record<string, number[]> : {}; } catch { return {}; } } @@ export function unlinkedDpiSlots(): number[] { const key = unlinkedKey(); - return key ? unlinkedStore()[key] ?? [] : []; + const saved = key ? unlinkedStore()[key] : undefined; + return Array.isArray(saved) ? saved : []; } export function setDpiSlotUnlinked(index: number, unlinked: boolean): void { const key = unlinkedKey(); if (!key) return; - const rows = new Set(unlinkedStore()[key] ?? []); + const store = unlinkedStore(); + const rows = new Set(Array.isArray(store[key]) ? store[key] : []); if (unlinked) rows.add(index); else rows.delete(index); try { - localStorage.setItem(DPI_UNLINKED_STORAGE_KEY, JSON.stringify({ ...unlinkedStore(), [key]: [...rows].sort((a, b) => a - b) })); + localStorage.setItem(DPI_UNLINKED_STORAGE_KEY, JSON.stringify({ ...store, [key]: [...rows].sort((a, b) => a - b) })); } catch { } }🤖 Prompt for AI Agents