refactor(graph): simplify comment particle rendering with dedicated bubble function
- Replaced inline drawing logic for task comments with a new `drawCommentBubble` function to enhance readability and maintainability. - The new function encapsulates the drawing of a speech-bubble icon, including the rounded rectangle body, tail, and inner dots to suggest text.
This commit is contained in:
parent
485327d077
commit
92968b45ad
9 changed files with 206 additions and 34 deletions
|
|
@ -156,16 +156,7 @@ function drawParticleCore(
|
||||||
ctx.drawImage(sprite, pos.x - glowR, pos.y - glowR);
|
ctx.drawImage(sprite, pos.x - glowR, pos.y - glowR);
|
||||||
|
|
||||||
if (kind === 'task_comment') {
|
if (kind === 'task_comment') {
|
||||||
ctx.strokeStyle = color;
|
drawCommentBubble(ctx, pos.x, pos.y, size, color);
|
||||||
ctx.lineWidth = 1.8;
|
|
||||||
ctx.beginPath();
|
|
||||||
ctx.arc(pos.x, pos.y, size * 1.1, 0, Math.PI * 2);
|
|
||||||
ctx.stroke();
|
|
||||||
|
|
||||||
ctx.fillStyle = '#ffffff';
|
|
||||||
ctx.beginPath();
|
|
||||||
ctx.arc(pos.x, pos.y, size * 0.35, 0, Math.PI * 2);
|
|
||||||
ctx.fill();
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -181,3 +172,48 @@ function drawParticleCore(
|
||||||
ctx.arc(pos.x, pos.y, size * PARTICLE_DRAW.coreHighlightScale, 0, Math.PI * 2);
|
ctx.arc(pos.x, pos.y, size * PARTICLE_DRAW.coreHighlightScale, 0, Math.PI * 2);
|
||||||
ctx.fill();
|
ctx.fill();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Draw a speech-bubble icon for comment particles.
|
||||||
|
* Rounded rect body + small triangular tail at bottom-left.
|
||||||
|
*/
|
||||||
|
function drawCommentBubble(
|
||||||
|
ctx: CanvasRenderingContext2D,
|
||||||
|
cx: number,
|
||||||
|
cy: number,
|
||||||
|
size: number,
|
||||||
|
color: string,
|
||||||
|
): void {
|
||||||
|
const w = size * 2.4;
|
||||||
|
const h = size * 1.8;
|
||||||
|
const r = size * 0.4; // corner radius
|
||||||
|
const x = cx - w / 2;
|
||||||
|
const y = cy - h / 2;
|
||||||
|
|
||||||
|
// Bubble body
|
||||||
|
ctx.fillStyle = color;
|
||||||
|
ctx.beginPath();
|
||||||
|
ctx.roundRect(x, y, w, h, r);
|
||||||
|
ctx.fill();
|
||||||
|
|
||||||
|
// Tail (small triangle at bottom-left)
|
||||||
|
const tailX = x + w * 0.25;
|
||||||
|
const tailY = y + h;
|
||||||
|
ctx.beginPath();
|
||||||
|
ctx.moveTo(tailX, tailY - 1);
|
||||||
|
ctx.lineTo(tailX - size * 0.4, tailY + size * 0.5);
|
||||||
|
ctx.lineTo(tailX + size * 0.4, tailY - 1);
|
||||||
|
ctx.closePath();
|
||||||
|
ctx.fill();
|
||||||
|
|
||||||
|
// Inner dots (three small dots to suggest text)
|
||||||
|
ctx.fillStyle = '#ffffff';
|
||||||
|
const dotR = size * 0.18;
|
||||||
|
const dotY = cy - size * 0.05;
|
||||||
|
const gap = size * 0.5;
|
||||||
|
for (let i = -1; i <= 1; i++) {
|
||||||
|
ctx.beginPath();
|
||||||
|
ctx.arc(cx + i * gap, dotY, dotR, 0, Math.PI * 2);
|
||||||
|
ctx.fill();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -371,6 +371,7 @@ export class ChangeExtractorService {
|
||||||
return derived.length > 0 ? derived : undefined;
|
return derived.length > 0 ? derived : undefined;
|
||||||
})();
|
})();
|
||||||
return {
|
return {
|
||||||
|
createdAt: typeof parsed.createdAt === 'string' ? parsed.createdAt : undefined,
|
||||||
owner: typeof parsed.owner === 'string' ? parsed.owner : undefined,
|
owner: typeof parsed.owner === 'string' ? parsed.owner : undefined,
|
||||||
status: typeof parsed.status === 'string' ? parsed.status : undefined,
|
status: typeof parsed.status === 'string' ? parsed.status : undefined,
|
||||||
intervals: derivedIntervals,
|
intervals: derivedIntervals,
|
||||||
|
|
@ -725,6 +726,7 @@ export class ChangeExtractorService {
|
||||||
}
|
}
|
||||||
|
|
||||||
const descriptor = buildTaskChangePresenceDescriptor({
|
const descriptor = buildTaskChangePresenceDescriptor({
|
||||||
|
createdAt: taskMeta.createdAt,
|
||||||
owner: effectiveOptions.owner ?? taskMeta.owner,
|
owner: effectiveOptions.owner ?? taskMeta.owner,
|
||||||
status: effectiveOptions.status ?? taskMeta.status,
|
status: effectiveOptions.status ?? taskMeta.status,
|
||||||
intervals: effectiveOptions.intervals ?? taskMeta.intervals,
|
intervals: effectiveOptions.intervals ?? taskMeta.intervals,
|
||||||
|
|
|
||||||
|
|
@ -234,6 +234,7 @@ export class TeamDataService {
|
||||||
|
|
||||||
for (const task of tasks) {
|
for (const task of tasks) {
|
||||||
const descriptor = buildTaskChangePresenceDescriptor({
|
const descriptor = buildTaskChangePresenceDescriptor({
|
||||||
|
createdAt: task.createdAt,
|
||||||
owner: task.owner,
|
owner: task.owner,
|
||||||
status: task.status,
|
status: task.status,
|
||||||
intervals: task.workIntervals,
|
intervals: task.workIntervals,
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,7 @@ import {
|
||||||
getTaskChangeStateBucket,
|
getTaskChangeStateBucket,
|
||||||
type TaskChangeStateBucket,
|
type TaskChangeStateBucket,
|
||||||
} from '@shared/utils/taskChangeState';
|
} from '@shared/utils/taskChangeState';
|
||||||
|
import { deriveTaskSince } from '@shared/utils/taskChangeSince';
|
||||||
import { createHash } from 'crypto';
|
import { createHash } from 'crypto';
|
||||||
|
|
||||||
export interface TaskChangePresenceInterval {
|
export interface TaskChangePresenceInterval {
|
||||||
|
|
@ -13,6 +14,7 @@ export interface TaskChangePresenceDescriptorInput {
|
||||||
owner?: string;
|
owner?: string;
|
||||||
status?: string;
|
status?: string;
|
||||||
intervals?: TaskChangePresenceInterval[];
|
intervals?: TaskChangePresenceInterval[];
|
||||||
|
createdAt?: string;
|
||||||
since?: string;
|
since?: string;
|
||||||
reviewState?: 'review' | 'needsFix' | 'approved' | 'none';
|
reviewState?: 'review' | 'needsFix' | 'approved' | 'none';
|
||||||
historyEvents?: unknown[];
|
historyEvents?: unknown[];
|
||||||
|
|
@ -102,6 +104,15 @@ export function computeTaskChangePresenceProjectFingerprint(
|
||||||
export function buildTaskChangePresenceDescriptor(
|
export function buildTaskChangePresenceDescriptor(
|
||||||
input: TaskChangePresenceDescriptorInput
|
input: TaskChangePresenceDescriptorInput
|
||||||
): TaskChangePresenceDescriptor {
|
): TaskChangePresenceDescriptor {
|
||||||
|
const effectiveSince =
|
||||||
|
typeof input.since === 'string'
|
||||||
|
? input.since
|
||||||
|
: deriveTaskSince({
|
||||||
|
createdAt: input.createdAt,
|
||||||
|
workIntervals: input.intervals,
|
||||||
|
historyEvents: input.historyEvents as { timestamp?: string | null }[] | undefined,
|
||||||
|
});
|
||||||
|
|
||||||
const effectiveIntervals =
|
const effectiveIntervals =
|
||||||
Array.isArray(input.intervals) && input.intervals.length > 0
|
Array.isArray(input.intervals) && input.intervals.length > 0
|
||||||
? input.intervals.map((interval) => ({
|
? input.intervals.map((interval) => ({
|
||||||
|
|
@ -124,7 +135,7 @@ export function buildTaskChangePresenceDescriptor(
|
||||||
owner: typeof input.owner === 'string' ? input.owner.trim() : '',
|
owner: typeof input.owner === 'string' ? input.owner.trim() : '',
|
||||||
status: typeof input.status === 'string' ? input.status.trim() : '',
|
status: typeof input.status === 'string' ? input.status.trim() : '',
|
||||||
intervals: effectiveIntervals,
|
intervals: effectiveIntervals,
|
||||||
since: typeof input.since === 'string' ? input.since : '',
|
since: effectiveSince ?? '',
|
||||||
};
|
};
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
import type { TaskChangeSetV2 } from '@shared/types';
|
import type { TaskChangeSetV2 } from '@shared/types';
|
||||||
|
|
||||||
export interface TaskChangeTaskMeta {
|
export interface TaskChangeTaskMeta {
|
||||||
|
createdAt?: string;
|
||||||
owner?: string;
|
owner?: string;
|
||||||
status?: string;
|
status?: string;
|
||||||
intervals?: { startedAt: string; completedAt?: string }[];
|
intervals?: { startedAt: string; completedAt?: string }[];
|
||||||
|
|
|
||||||
|
|
@ -494,6 +494,9 @@ export class TeamGraphAdapter {
|
||||||
if (this.#seenMessageIds.has(msgKey)) continue;
|
if (this.#seenMessageIds.has(msgKey)) continue;
|
||||||
this.#seenMessageIds.add(msgKey);
|
this.#seenMessageIds.add(msgKey);
|
||||||
|
|
||||||
|
// Skip comment notifications — #buildCommentParticles handles them with real text
|
||||||
|
if (msg.summary?.startsWith('Comment on ')) continue;
|
||||||
|
|
||||||
const edgeId = TeamGraphAdapter.#resolveMessageEdge(msg, teamName, leadId, leadName, edges);
|
const edgeId = TeamGraphAdapter.#resolveMessageEdge(msg, teamName, leadId, leadName, edges);
|
||||||
if (!edgeId) continue;
|
if (!edgeId) continue;
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -3,12 +3,11 @@ import {
|
||||||
isTaskChangeSummaryCacheable,
|
isTaskChangeSummaryCacheable,
|
||||||
type TaskChangeStateBucket,
|
type TaskChangeStateBucket,
|
||||||
} from '@shared/utils/taskChangeState';
|
} from '@shared/utils/taskChangeState';
|
||||||
|
import { deriveTaskSince as deriveSharedTaskSince } from '@shared/utils/taskChangeSince';
|
||||||
|
|
||||||
import type { ReviewAPI } from '@shared/types/api';
|
import type { ReviewAPI } from '@shared/types/api';
|
||||||
import type { TeamTaskWithKanban } from '@shared/types/team';
|
import type { TeamTaskWithKanban } from '@shared/types/team';
|
||||||
|
|
||||||
const TASK_SINCE_GRACE_MS = 2 * 60 * 1000;
|
|
||||||
|
|
||||||
export type TaskChangeRequestOptions = NonNullable<Parameters<ReviewAPI['getTaskChanges']>[2]>;
|
export type TaskChangeRequestOptions = NonNullable<Parameters<ReviewAPI['getTaskChanges']>[2]>;
|
||||||
|
|
||||||
export interface TaskChangeContext {
|
export interface TaskChangeContext {
|
||||||
|
|
@ -31,27 +30,7 @@ type TaskChangeTaskLike = Pick<
|
||||||
>;
|
>;
|
||||||
|
|
||||||
export function deriveTaskSince(task: TaskChangeTaskLike | null): string | undefined {
|
export function deriveTaskSince(task: TaskChangeTaskLike | null): string | undefined {
|
||||||
if (!task) return undefined;
|
return deriveSharedTaskSince(task);
|
||||||
|
|
||||||
const sources: string[] = [];
|
|
||||||
if (task.createdAt) sources.push(task.createdAt);
|
|
||||||
if (Array.isArray(task.workIntervals)) {
|
|
||||||
for (const interval of task.workIntervals) {
|
|
||||||
if (interval.startedAt) sources.push(interval.startedAt);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (Array.isArray(task.historyEvents)) {
|
|
||||||
for (const event of task.historyEvents) {
|
|
||||||
if (event.timestamp) sources.push(event.timestamp);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (sources.length === 0) return undefined;
|
|
||||||
|
|
||||||
const [first, ...rest] = sources;
|
|
||||||
const earliest = rest.reduce((a, b) => (a < b ? a : b), first);
|
|
||||||
const date = new Date(earliest);
|
|
||||||
date.setTime(date.getTime() - TASK_SINCE_GRACE_MS);
|
|
||||||
return date.toISOString();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export function buildTaskChangeRequestOptions(
|
export function buildTaskChangeRequestOptions(
|
||||||
|
|
|
||||||
51
src/shared/utils/taskChangeSince.ts
Normal file
51
src/shared/utils/taskChangeSince.ts
Normal file
|
|
@ -0,0 +1,51 @@
|
||||||
|
const TASK_SINCE_GRACE_MS = 2 * 60 * 1000;
|
||||||
|
|
||||||
|
type TaskChangeIntervalLike = {
|
||||||
|
startedAt?: string | null;
|
||||||
|
};
|
||||||
|
|
||||||
|
type TaskChangeHistoryEventLike = {
|
||||||
|
timestamp?: string | null;
|
||||||
|
};
|
||||||
|
|
||||||
|
export interface TaskChangeSinceLike<
|
||||||
|
TInterval extends TaskChangeIntervalLike = TaskChangeIntervalLike,
|
||||||
|
THistoryEvent extends TaskChangeHistoryEventLike = TaskChangeHistoryEventLike,
|
||||||
|
> {
|
||||||
|
createdAt?: string | null;
|
||||||
|
workIntervals?: TInterval[] | null;
|
||||||
|
historyEvents?: THistoryEvent[] | null;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function deriveTaskSince<
|
||||||
|
TInterval extends TaskChangeIntervalLike,
|
||||||
|
THistoryEvent extends TaskChangeHistoryEventLike,
|
||||||
|
>(task: TaskChangeSinceLike<TInterval, THistoryEvent> | null | undefined): string | undefined {
|
||||||
|
if (!task) return undefined;
|
||||||
|
|
||||||
|
const sources: string[] = [];
|
||||||
|
if (typeof task.createdAt === 'string' && task.createdAt.length > 0) {
|
||||||
|
sources.push(task.createdAt);
|
||||||
|
}
|
||||||
|
if (Array.isArray(task.workIntervals)) {
|
||||||
|
for (const interval of task.workIntervals) {
|
||||||
|
if (typeof interval?.startedAt === 'string' && interval.startedAt.length > 0) {
|
||||||
|
sources.push(interval.startedAt);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (Array.isArray(task.historyEvents)) {
|
||||||
|
for (const event of task.historyEvents) {
|
||||||
|
if (typeof event?.timestamp === 'string' && event.timestamp.length > 0) {
|
||||||
|
sources.push(event.timestamp);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (sources.length === 0) return undefined;
|
||||||
|
|
||||||
|
const [first, ...rest] = sources;
|
||||||
|
const earliest = rest.reduce((a, b) => (a < b ? a : b), first);
|
||||||
|
const date = new Date(earliest);
|
||||||
|
date.setTime(date.getTime() - TASK_SINCE_GRACE_MS);
|
||||||
|
return date.toISOString();
|
||||||
|
}
|
||||||
|
|
@ -1867,6 +1867,94 @@ describe('TeamDataService', () => {
|
||||||
expect(mismatched.tasks[0]?.changePresence).toBe('unknown');
|
expect(mismatched.tasks[0]?.changePresence).toBe('unknown');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('preserves cached changePresence when persisted entry was recorded with derived since', async () => {
|
||||||
|
const task: TeamTask = {
|
||||||
|
id: 'task-1',
|
||||||
|
subject: 'Review API',
|
||||||
|
status: 'completed',
|
||||||
|
owner: 'alice',
|
||||||
|
createdAt: '2026-03-01T10:05:00.000Z',
|
||||||
|
workIntervals: [{ startedAt: '2026-03-01T10:10:00.000Z' }],
|
||||||
|
historyEvents: [
|
||||||
|
{
|
||||||
|
id: 'evt-1',
|
||||||
|
type: 'status_changed',
|
||||||
|
from: 'pending',
|
||||||
|
to: 'in_progress',
|
||||||
|
timestamp: '2026-03-01T10:00:00.000Z',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
};
|
||||||
|
|
||||||
|
const persistedDescriptor = buildTaskChangePresenceDescriptor({
|
||||||
|
createdAt: task.createdAt,
|
||||||
|
owner: task.owner,
|
||||||
|
status: task.status,
|
||||||
|
intervals: task.workIntervals,
|
||||||
|
since: '2026-03-01T09:58:00.000Z',
|
||||||
|
historyEvents: task.historyEvents,
|
||||||
|
reviewState: 'none',
|
||||||
|
});
|
||||||
|
|
||||||
|
const service = new TeamDataService(
|
||||||
|
{
|
||||||
|
listTeams: vi.fn(),
|
||||||
|
getConfig: vi.fn(async () => ({ name: 'My team', members: [], projectPath: '/repo' })),
|
||||||
|
} as never,
|
||||||
|
{
|
||||||
|
getTasks: vi.fn(async () => [task]),
|
||||||
|
} as never,
|
||||||
|
{
|
||||||
|
listInboxNames: vi.fn(async () => []),
|
||||||
|
getMessages: vi.fn(async () => []),
|
||||||
|
} as never,
|
||||||
|
{} as never,
|
||||||
|
{} as never,
|
||||||
|
{
|
||||||
|
resolveMembers: vi.fn(() => []),
|
||||||
|
} as never,
|
||||||
|
{
|
||||||
|
getState: vi.fn(async () => ({ teamName: 'my-team', reviewers: [], tasks: {} })),
|
||||||
|
} as never
|
||||||
|
);
|
||||||
|
|
||||||
|
service.setTaskChangePresenceServices(
|
||||||
|
{
|
||||||
|
load: vi.fn(async () => ({
|
||||||
|
version: 1,
|
||||||
|
teamName: 'my-team',
|
||||||
|
projectFingerprint: 'project-fingerprint',
|
||||||
|
logSourceGeneration: 'log-generation',
|
||||||
|
writtenAt: '2026-03-01T12:00:00.000Z',
|
||||||
|
entries: {
|
||||||
|
'task-1': {
|
||||||
|
taskId: 'task-1',
|
||||||
|
taskSignature: persistedDescriptor.taskSignature,
|
||||||
|
presence: 'has_changes',
|
||||||
|
writtenAt: '2026-03-01T12:00:00.000Z',
|
||||||
|
logSourceGeneration: 'log-generation',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})),
|
||||||
|
upsertEntry: vi.fn(async () => undefined),
|
||||||
|
} as never,
|
||||||
|
{
|
||||||
|
getSnapshot: vi.fn(() => ({
|
||||||
|
projectFingerprint: 'project-fingerprint',
|
||||||
|
logSourceGeneration: 'log-generation',
|
||||||
|
})),
|
||||||
|
ensureTracking: vi.fn(async () => ({
|
||||||
|
projectFingerprint: 'project-fingerprint',
|
||||||
|
logSourceGeneration: 'log-generation',
|
||||||
|
})),
|
||||||
|
} as never
|
||||||
|
);
|
||||||
|
|
||||||
|
const data = await service.getTeamData('my-team');
|
||||||
|
|
||||||
|
expect(data.tasks[0]?.changePresence).toBe('has_changes');
|
||||||
|
});
|
||||||
|
|
||||||
it('returns lightweight task change presence without loading full team data', async () => {
|
it('returns lightweight task change presence without loading full team data', async () => {
|
||||||
const task: TeamTask = {
|
const task: TeamTask = {
|
||||||
id: 'task-1',
|
id: 'task-1',
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue