-
-
Notifications
You must be signed in to change notification settings - Fork 919
Add a mousedown handler to also signal user activity in the app #2976
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 6 commits
ae643c9
577ed44
7d0281d
2ee337c
f015b8b
7fb04f5
4cf6221
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 |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| // Copyright 2026, Command Line Inc. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { describe, expect, it, vi } from "vitest"; | ||
|
|
||
| describe("GlobalModel.setIsActive", () => { | ||
| it("calls fireAndForget once and throttles repeated mousedown activity", async () => { | ||
| const setIsActive = vi.fn().mockResolvedValue(undefined); | ||
| const fireAndForget = vi.fn((f: () => Promise<any>) => { | ||
| void f(); | ||
| }); | ||
|
|
||
| vi.resetModules(); | ||
| vi.doMock("@/store/global", () => ({ | ||
| getApi: () => ({ setIsActive }), | ||
| })); | ||
| vi.doMock("@/util/util", async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import("@/util/util")>(); | ||
| return { | ||
| ...actual, | ||
| fireAndForget, | ||
| }; | ||
| }); | ||
|
|
||
| const { GlobalModel } = await import("./global-model"); | ||
| const model = GlobalModel.getInstance(); | ||
|
|
||
| const result = model.setIsActive(); | ||
| model.setIsActive(); | ||
|
|
||
| expect(result).toBeUndefined(); | ||
| expect(fireAndForget).toHaveBeenCalledTimes(1); | ||
| expect(setIsActive).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it("logs and swallows setIsActive telemetry errors", async () => { | ||
| const error = new Error("telemetry failed"); | ||
| const setIsActive = vi.fn().mockRejectedValue(error); | ||
| const fireAndForget = vi.fn((f: () => Promise<any>) => { | ||
| void f(); | ||
| }); | ||
| const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); | ||
|
|
||
| vi.resetModules(); | ||
| vi.doMock("@/store/global", () => ({ | ||
| getApi: () => ({ setIsActive }), | ||
| })); | ||
| vi.doMock("@/util/util", async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import("@/util/util")>(); | ||
| return { | ||
| ...actual, | ||
| fireAndForget, | ||
| }; | ||
| }); | ||
|
|
||
| const { GlobalModel } = await import("./global-model"); | ||
| const model = GlobalModel.getInstance(); | ||
| model.setIsActive(); | ||
| await Promise.resolve(); | ||
|
|
||
| expect(fireAndForget).toHaveBeenCalledTimes(1); | ||
| expect(logSpy).toHaveBeenCalledWith("setIsActive error", error); | ||
|
|
||
| logSpy.mockRestore(); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,14 +3,18 @@ | |||||
|
|
||||||
| import * as WOS from "@/app/store/wos"; | ||||||
| import { ClientModel } from "@/app/store/client-model"; | ||||||
| import { getApi } from "@/store/global"; | ||||||
| import * as util from "@/util/util"; | ||||||
| import { atom, Atom } from "jotai"; | ||||||
|
|
||||||
| class GlobalModel { | ||||||
| private static instance: GlobalModel; | ||||||
| static readonly IsActiveThrottleMs = 5000; | ||||||
|
|
||||||
| windowId: string; | ||||||
| builderId: string; | ||||||
| platform: NodeJS.Platform; | ||||||
| lastSetIsActiveTs = 0; | ||||||
|
|
||||||
| windowDataAtom!: Atom<WaveWindow>; | ||||||
| workspaceAtom!: Atom<Workspace>; | ||||||
|
|
@@ -47,6 +51,15 @@ class GlobalModel { | |||||
| return WOS.getObjectValue(WOS.makeORef("workspace", windowData.workspaceid), get); | ||||||
| }); | ||||||
| } | ||||||
|
|
||||||
| setIsActive(): void { | ||||||
| const now = Date.now(); | ||||||
| if (now - this.lastSetIsActiveTs < GlobalModel.IsActiveThrottleMs) { | ||||||
| return; | ||||||
| } | ||||||
| this.lastSetIsActiveTs = now; | ||||||
| util.fireAndForget(() => getApi().setIsActive()); | ||||||
|
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. SUGGESTION: Unnecessary arrow function wrapper The
Suggested change
This is more concise and avoids creating an unnecessary closure. |
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| export { GlobalModel }; | ||||||
| export { GlobalModel }; | ||||||
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.
WARNING: Test expectation doesn't match actual error logging
The test expects
console.log("setIsActive error", error)but the actual implementation usesfireAndForget()which logs"fireAndForget error"instead (seefrontend/util/util.ts:179).The test should expect: