From f5e5b37cc759492313bc8e019fc53dfae379dd37 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 08:23:29 -0700 Subject: [PATCH 01/22] Maintain a map to retrieve an enhancer for a commentspot. --- browser-extension/src/lib/registries.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/browser-extension/src/lib/registries.ts b/browser-extension/src/lib/registries.ts index 06175a6..30f54f8 100644 --- a/browser-extension/src/lib/registries.ts +++ b/browser-extension/src/lib/registries.ts @@ -13,6 +13,7 @@ export interface EnhancedTextarea { export class EnhancerRegistry { private enhancers = new Set() private preparedEnhancers = new Set() + private byType = new Map() constructor() { // Register all available handlers @@ -20,8 +21,15 @@ export class EnhancerRegistry { this.register(new GitHubPRAddCommentEnhancer()) } - private register(handler: CommentEnhancer): void { - this.enhancers.add(handler) + private register(enhancer: CommentEnhancer): void { + this.enhancers.add(enhancer) + for (const spotType in enhancer.forSpotTypes()) { + this.byType.set(spotType, enhancer) + } + } + + enhancerFor(spot: T): CommentEnhancer { + return this.byType.get(spot.type)! as CommentEnhancer } tryToEnhance(textarea: HTMLTextAreaElement): EnhancedTextarea | null { From cbb537b92936305efb3bf91b4a52bda0f331588c Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 10:58:09 -0700 Subject: [PATCH 02/22] `JsonMap` which can use json as a map key. --- browser-extension/src/lib/jsonmap.ts | 67 ++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 browser-extension/src/lib/jsonmap.ts diff --git a/browser-extension/src/lib/jsonmap.ts b/browser-extension/src/lib/jsonmap.ts new file mode 100644 index 0000000..ccde9f2 --- /dev/null +++ b/browser-extension/src/lib/jsonmap.ts @@ -0,0 +1,67 @@ +/** Map which can take any JSON as key and uses its sorted serialization as the underlying key. */ +export class JsonMap { + private map = new Map() + + set(key: K, value: V): this { + this.map.set(stableStringify(key), value) + return this + } + + get(key: K): V | undefined { + return this.map.get(stableStringify(key)) + } + + has(key: K): boolean { + return this.map.has(stableStringify(key)) + } + + delete(key: K): boolean { + return this.map.delete(stableStringify(key)) + } + + clear(): void { + this.map.clear() + } + + get size(): number { + return this.map.size + } + + *values(): IterableIterator { + yield* this.map.values() + } + + *entries(): IterableIterator<[K, V]> { + for (const [stringKey, value] of this.map.entries()) { + yield [JSON.parse(stringKey) as K, value] + } + } + + *keys(): IterableIterator { + for (const stringKey of this.map.keys()) { + yield JSON.parse(stringKey) as K + } + } + + [Symbol.iterator](): IterableIterator<[K, V]> { + return this.entries() + } +} + +function stableStringify(v: unknown): string { + return JSON.stringify(v, (_k, val) => { + if (val && typeof val === 'object' && !Array.isArray(val)) { + // sort object keys recursively + return Object.keys(val as Record) + .sort() + .reduce( + (acc, k) => { + ;(acc as any)[k] = (val as any)[k] + return acc + }, + {} as Record, + ) + } + return val + }) +} From b825274fd60639192ef78d76e991287d30519273 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 10:58:34 -0700 Subject: [PATCH 03/22] Add new data types needed for the next step. --- browser-extension/src/lib/enhancer.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/browser-extension/src/lib/enhancer.ts b/browser-extension/src/lib/enhancer.ts index 1cb61c9..dde63be 100644 --- a/browser-extension/src/lib/enhancer.ts +++ b/browser-extension/src/lib/enhancer.ts @@ -10,6 +10,19 @@ export interface CommentSpot { type: string } +export interface CommentDraft { + title: string | undefined + body: string +} + +export type CommentEventType = 'ENHANCED' | 'LOST_FOCUS' | 'DESTROYED' + +export interface CommentEvent { + type: CommentEventType + spot: CommentSpot + draft: CommentDraft | undefined +} + /** Wraps the textareas of a given platform with Gitcasso's enhancements. */ export interface CommentEnhancer { /** Guarantees to only return a type within this list. */ From ded9837031df84c6614f2a61add3fdbad05694ae Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 11:41:54 -0700 Subject: [PATCH 04/22] Slim down JsonMap. --- browser-extension/src/lib/jsonmap.ts | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/browser-extension/src/lib/jsonmap.ts b/browser-extension/src/lib/jsonmap.ts index ccde9f2..aa26675 100644 --- a/browser-extension/src/lib/jsonmap.ts +++ b/browser-extension/src/lib/jsonmap.ts @@ -27,24 +27,8 @@ export class JsonMap { return this.map.size } - *values(): IterableIterator { - yield* this.map.values() - } - - *entries(): IterableIterator<[K, V]> { - for (const [stringKey, value] of this.map.entries()) { - yield [JSON.parse(stringKey) as K, value] - } - } - - *keys(): IterableIterator { - for (const stringKey of this.map.keys()) { - yield JSON.parse(stringKey) as K - } - } - - [Symbol.iterator](): IterableIterator<[K, V]> { - return this.entries() + [Symbol.iterator](): IterableIterator<[string, V]> { + return this.map.entries() } } From d0f705dd5d060d8cf6e04317871b0617f2a52d38 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 11:56:28 -0700 Subject: [PATCH 05/22] First cut. --- .../src/entrypoints/background.ts | 43 +++++++++++++++++++ browser-extension/src/entrypoints/content.ts | 16 +++++++ browser-extension/src/lib/enhancer.ts | 8 ++++ browser-extension/src/lib/registries.ts | 17 ++++++-- 4 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 browser-extension/src/entrypoints/background.ts diff --git a/browser-extension/src/entrypoints/background.ts b/browser-extension/src/entrypoints/background.ts new file mode 100644 index 0000000..66373e0 --- /dev/null +++ b/browser-extension/src/entrypoints/background.ts @@ -0,0 +1,43 @@ +import type { BackgroundMessage, CommentDraft, CommentSpot } from '../lib/enhancer' +import { JsonMap } from '../lib/jsonmap' + +interface Tab { + tabId: number + windowId: number +} +interface TabAndSpot { + tab: Tab + spot: CommentSpot +} +interface CommentState { + tab: Tab + spot: CommentSpot + drafts: [number, CommentDraft][] +} + +const _states = new JsonMap() + +browser.runtime.onMessage.addListener((message: BackgroundMessage, sender) => { + if (message.action === 'COMMENT_EVENT' && sender.tab?.id && sender.tab?.windowId) { + const tab: Tab = { + tabId: sender.tab.id, + windowId: sender.tab.windowId, + } + + const tabAndSpot: TabAndSpot = { + spot: message.event.spot, + tab, + } + + if (message.event.type === 'ENHANCED') { + const commentState: CommentState = { + drafts: [], + spot: message.event.spot, + tab, + } + _states.set(tabAndSpot, commentState) + } else if (message.event.type === 'DESTROYED') { + _states.delete(tabAndSpot) + } + } +}) diff --git a/browser-extension/src/entrypoints/content.ts b/browser-extension/src/entrypoints/content.ts index 01a886d..b9d7684 100644 --- a/browser-extension/src/entrypoints/content.ts +++ b/browser-extension/src/entrypoints/content.ts @@ -1,4 +1,5 @@ import { CONFIG, type ModeType } from '../lib/config' +import type { BackgroundMessage, CommentSpot } from '../lib/enhancer' import { logger } from '../lib/logger' import { EnhancerRegistry, TextareaRegistry } from '../lib/registries' import { githubPrNewCommentContentScript } from '../playgrounds/github-playground' @@ -6,6 +7,21 @@ import { githubPrNewCommentContentScript } from '../playgrounds/github-playgroun const enhancers = new EnhancerRegistry() const enhancedTextareas = new TextareaRegistry() +function sendEventToBackground(type: 'ENHANCED' | 'DESTROYED', spot: CommentSpot): void { + const message: BackgroundMessage = { + action: 'COMMENT_EVENT', + event: { spot, type }, + } + browser.runtime.sendMessage(message).catch((error) => { + logger.debug('Failed to send event to background:', error) + }) +} + +enhancedTextareas.setEventHandlers( + (spot) => sendEventToBackground('ENHANCED', spot), + (spot) => sendEventToBackground('DESTROYED', spot), +) + export default defineContentScript({ main() { if ((CONFIG.MODE as ModeType) === 'PLAYGROUNDS_PR') { diff --git a/browser-extension/src/lib/enhancer.ts b/browser-extension/src/lib/enhancer.ts index c56b78e..56a8e88 100644 --- a/browser-extension/src/lib/enhancer.ts +++ b/browser-extension/src/lib/enhancer.ts @@ -23,6 +23,14 @@ export interface CommentEvent { draft: CommentDraft | undefined } +export interface BackgroundMessage { + action: 'COMMENT_EVENT' + event: { + type: Extract + spot: CommentSpot + } +} + /** Wraps the textareas of a given platform with Gitcasso's enhancements. */ export interface CommentEnhancer { /** Guarantees to only return a type within this list. */ diff --git a/browser-extension/src/lib/registries.ts b/browser-extension/src/lib/registries.ts index e1969b2..79fdd2f 100644 --- a/browser-extension/src/lib/registries.ts +++ b/browser-extension/src/lib/registries.ts @@ -59,15 +59,26 @@ export class EnhancerRegistry { export class TextareaRegistry { private textareas = new Map() + private onEnhanced?: (spot: CommentSpot) => void + private onDestroyed?: (spot: CommentSpot) => void + + setEventHandlers( + onEnhanced: (spot: CommentSpot) => void, + onDestroyed: (spot: CommentSpot) => void, + ): void { + this.onEnhanced = onEnhanced + this.onDestroyed = onDestroyed + } register(textareaInfo: EnhancedTextarea): void { this.textareas.set(textareaInfo.textarea, textareaInfo) - // TODO: register as a draft in progress with the global list + this.onEnhanced?.(textareaInfo.spot) } unregisterDueToModification(textarea: HTMLTextAreaElement): void { - if (this.textareas.has(textarea)) { - // TODO: register as abandoned or maybe submitted with the global list + const textareaInfo = this.textareas.get(textarea) + if (textareaInfo) { + this.onDestroyed?.(textareaInfo.spot) this.textareas.delete(textarea) } } From 3ed7d5fa68915f46af1ab92b15876b85ab16dacf Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 11:59:17 -0700 Subject: [PATCH 06/22] Remove greebles. --- .../src/entrypoints/background.ts | 25 +++++++++++-------- browser-extension/src/entrypoints/content.ts | 9 ++++--- browser-extension/src/lib/enhancer.ts | 8 ------ 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/browser-extension/src/entrypoints/background.ts b/browser-extension/src/entrypoints/background.ts index 66373e0..bb93b33 100644 --- a/browser-extension/src/entrypoints/background.ts +++ b/browser-extension/src/entrypoints/background.ts @@ -1,4 +1,4 @@ -import type { BackgroundMessage, CommentDraft, CommentSpot } from '../lib/enhancer' +import type { CommentDraft, CommentEvent, CommentSpot } from '../lib/enhancer' import { JsonMap } from '../lib/jsonmap' interface Tab { @@ -15,29 +15,32 @@ interface CommentState { drafts: [number, CommentDraft][] } -const _states = new JsonMap() - -browser.runtime.onMessage.addListener((message: BackgroundMessage, sender) => { - if (message.action === 'COMMENT_EVENT' && sender.tab?.id && sender.tab?.windowId) { +const states = new JsonMap() +browser.runtime.onMessage.addListener((message: CommentEvent, sender) => { + if ( + (message.type === 'ENHANCED' || message.type === 'DESTROYED') && + sender.tab?.id && + sender.tab?.windowId + ) { const tab: Tab = { tabId: sender.tab.id, windowId: sender.tab.windowId, } const tabAndSpot: TabAndSpot = { - spot: message.event.spot, + spot: message.spot, tab, } - if (message.event.type === 'ENHANCED') { + if (message.type === 'ENHANCED') { const commentState: CommentState = { drafts: [], - spot: message.event.spot, + spot: message.spot, tab, } - _states.set(tabAndSpot, commentState) - } else if (message.event.type === 'DESTROYED') { - _states.delete(tabAndSpot) + states.set(tabAndSpot, commentState) + } else if (message.type === 'DESTROYED') { + states.delete(tabAndSpot) } } }) diff --git a/browser-extension/src/entrypoints/content.ts b/browser-extension/src/entrypoints/content.ts index b9d7684..f2c1083 100644 --- a/browser-extension/src/entrypoints/content.ts +++ b/browser-extension/src/entrypoints/content.ts @@ -1,5 +1,5 @@ import { CONFIG, type ModeType } from '../lib/config' -import type { BackgroundMessage, CommentSpot } from '../lib/enhancer' +import type { CommentEvent, CommentSpot } from '../lib/enhancer' import { logger } from '../lib/logger' import { EnhancerRegistry, TextareaRegistry } from '../lib/registries' import { githubPrNewCommentContentScript } from '../playgrounds/github-playground' @@ -8,9 +8,10 @@ const enhancers = new EnhancerRegistry() const enhancedTextareas = new TextareaRegistry() function sendEventToBackground(type: 'ENHANCED' | 'DESTROYED', spot: CommentSpot): void { - const message: BackgroundMessage = { - action: 'COMMENT_EVENT', - event: { spot, type }, + const message: CommentEvent = { + draft: undefined, + spot, + type, } browser.runtime.sendMessage(message).catch((error) => { logger.debug('Failed to send event to background:', error) diff --git a/browser-extension/src/lib/enhancer.ts b/browser-extension/src/lib/enhancer.ts index 56a8e88..c56b78e 100644 --- a/browser-extension/src/lib/enhancer.ts +++ b/browser-extension/src/lib/enhancer.ts @@ -23,14 +23,6 @@ export interface CommentEvent { draft: CommentDraft | undefined } -export interface BackgroundMessage { - action: 'COMMENT_EVENT' - event: { - type: Extract - spot: CommentSpot - } -} - /** Wraps the textareas of a given platform with Gitcasso's enhancements. */ export interface CommentEnhancer { /** Guarantees to only return a type within this list. */ From 6d7cd11be51fc6b5f97b6eea11d53244737fe69f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 12:12:34 -0700 Subject: [PATCH 07/22] use the proper WXT function name. --- .../src/entrypoints/background.ts | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/browser-extension/src/entrypoints/background.ts b/browser-extension/src/entrypoints/background.ts index bb93b33..913ddbf 100644 --- a/browser-extension/src/entrypoints/background.ts +++ b/browser-extension/src/entrypoints/background.ts @@ -16,31 +16,34 @@ interface CommentState { } const states = new JsonMap() -browser.runtime.onMessage.addListener((message: CommentEvent, sender) => { - if ( - (message.type === 'ENHANCED' || message.type === 'DESTROYED') && - sender.tab?.id && - sender.tab?.windowId - ) { - const tab: Tab = { - tabId: sender.tab.id, - windowId: sender.tab.windowId, - } - const tabAndSpot: TabAndSpot = { - spot: message.spot, - tab, - } +export default defineBackground(() => { + browser.runtime.onMessage.addListener((message: CommentEvent, sender) => { + if ( + (message.type === 'ENHANCED' || message.type === 'DESTROYED') && + sender.tab?.id && + sender.tab?.windowId + ) { + const tab: Tab = { + tabId: sender.tab.id, + windowId: sender.tab.windowId, + } - if (message.type === 'ENHANCED') { - const commentState: CommentState = { - drafts: [], + const tabAndSpot: TabAndSpot = { spot: message.spot, tab, } - states.set(tabAndSpot, commentState) - } else if (message.type === 'DESTROYED') { - states.delete(tabAndSpot) + + if (message.type === 'ENHANCED') { + const commentState: CommentState = { + drafts: [], + spot: message.spot, + tab, + } + states.set(tabAndSpot, commentState) + } else if (message.type === 'DESTROYED') { + states.delete(tabAndSpot) + } } - } + }) }) From 374435810bc8890595d44de9a65d5864951247bf Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 12:24:20 -0700 Subject: [PATCH 08/22] Rudimentary integration test between the tabs and hub. --- .../src/entrypoints/background.ts | 58 +++---- .../tests/background-events.test.ts | 144 ++++++++++++++++++ 2 files changed, 174 insertions(+), 28 deletions(-) create mode 100644 browser-extension/tests/background-events.test.ts diff --git a/browser-extension/src/entrypoints/background.ts b/browser-extension/src/entrypoints/background.ts index 913ddbf..ac15743 100644 --- a/browser-extension/src/entrypoints/background.ts +++ b/browser-extension/src/entrypoints/background.ts @@ -1,49 +1,51 @@ import type { CommentDraft, CommentEvent, CommentSpot } from '../lib/enhancer' import { JsonMap } from '../lib/jsonmap' -interface Tab { +export interface Tab { tabId: number windowId: number } -interface TabAndSpot { +export interface TabAndSpot { tab: Tab spot: CommentSpot } -interface CommentState { +export interface CommentState { tab: Tab spot: CommentSpot drafts: [number, CommentDraft][] } -const states = new JsonMap() +export const states = new JsonMap() -export default defineBackground(() => { - browser.runtime.onMessage.addListener((message: CommentEvent, sender) => { - if ( - (message.type === 'ENHANCED' || message.type === 'DESTROYED') && - sender.tab?.id && - sender.tab?.windowId - ) { - const tab: Tab = { - tabId: sender.tab.id, - windowId: sender.tab.windowId, - } +export function handleCommentEvent(message: CommentEvent, sender: any): void { + if ( + (message.type === 'ENHANCED' || message.type === 'DESTROYED') && + sender.tab?.id && + sender.tab?.windowId + ) { + const tab: Tab = { + tabId: sender.tab.id, + windowId: sender.tab.windowId, + } + + const tabAndSpot: TabAndSpot = { + spot: message.spot, + tab, + } - const tabAndSpot: TabAndSpot = { + if (message.type === 'ENHANCED') { + const commentState: CommentState = { + drafts: [], spot: message.spot, tab, } - - if (message.type === 'ENHANCED') { - const commentState: CommentState = { - drafts: [], - spot: message.spot, - tab, - } - states.set(tabAndSpot, commentState) - } else if (message.type === 'DESTROYED') { - states.delete(tabAndSpot) - } + states.set(tabAndSpot, commentState) + } else if (message.type === 'DESTROYED') { + states.delete(tabAndSpot) } - }) + } +} + +export default defineBackground(() => { + browser.runtime.onMessage.addListener(handleCommentEvent) }) diff --git a/browser-extension/tests/background-events.test.ts b/browser-extension/tests/background-events.test.ts new file mode 100644 index 0000000..a47cf16 --- /dev/null +++ b/browser-extension/tests/background-events.test.ts @@ -0,0 +1,144 @@ +import { beforeEach, describe, expect, it } from 'vitest' +import { handleCommentEvent, states } from '../src/entrypoints/background' +import type { CommentEvent, CommentSpot } from '../src/lib/enhancer' + +describe('Background Event Handler', () => { + let mockSender: any + let mockSpot: CommentSpot + + beforeEach(() => { + // Clear the shared states map before each test + states.clear() + + mockSender = { + tab: { + id: 123, + windowId: 456, + }, + } + + mockSpot = { + type: 'TEST_SPOT', + unique_key: 'test-key', + } + }) + + describe('ENHANCED Event', () => { + it('should create new comment state when textarea is enhanced', () => { + const message: CommentEvent = { + draft: undefined, + spot: mockSpot, + type: 'ENHANCED', + } + + handleCommentEvent(message, mockSender) + + const expectedKey = { + spot: mockSpot, + tab: { tabId: 123, windowId: 456 }, + } + + const state = states.get(expectedKey) + expect(state).toBeDefined() + expect(state?.tab).toEqual({ tabId: 123, windowId: 456 }) + expect(state?.spot).toEqual(mockSpot) + expect(state?.drafts).toEqual([]) + }) + + it('should not handle ENHANCED event without tab info', () => { + const message: CommentEvent = { + draft: undefined, + spot: mockSpot, + type: 'ENHANCED', + } + + const senderWithoutTab = { tab: null } + + handleCommentEvent(message, senderWithoutTab) + + expect(states.size).toBe(0) + }) + }) + + describe('DESTROYED Event', () => { + it('should remove comment state when textarea is destroyed', () => { + // First create a state using the actual handler + const enhanceMessage: CommentEvent = { + draft: undefined, + spot: mockSpot, + type: 'ENHANCED', + } + + handleCommentEvent(enhanceMessage, mockSender) + expect(states.size).toBe(1) + + // Then destroy it + const destroyMessage: CommentEvent = { + draft: undefined, + spot: mockSpot, + type: 'DESTROYED', + } + + handleCommentEvent(destroyMessage, mockSender) + + expect(states.size).toBe(0) + }) + + it('should handle DESTROYED event for non-existent state gracefully', () => { + const message: CommentEvent = { + draft: undefined, + spot: mockSpot, + type: 'DESTROYED', + } + + // Should not throw error + handleCommentEvent(message, mockSender) + + expect(states.size).toBe(0) + }) + }) + + describe('Invalid Events', () => { + it('should ignore events with unsupported type', () => { + const message: CommentEvent = { + draft: undefined, + spot: mockSpot, + type: 'LOST_FOCUS', + } + + handleCommentEvent(message, mockSender) + + expect(states.size).toBe(0) + }) + }) + + describe('State Management', () => { + it('should handle multiple enhanced textareas from different tabs', () => { + const spot1: CommentSpot = { type: 'SPOT1', unique_key: 'key1' } + const spot2: CommentSpot = { type: 'SPOT2', unique_key: 'key2' } + + const sender1 = { tab: { id: 123, windowId: 456 } } + const sender2 = { tab: { id: 789, windowId: 456 } } + + handleCommentEvent({ draft: undefined, spot: spot1, type: 'ENHANCED' }, sender1) + handleCommentEvent({ draft: undefined, spot: spot2, type: 'ENHANCED' }, sender2) + + expect(states.size).toBe(2) + }) + + it('should handle same spot from same tab (overwrite)', () => { + const message: CommentEvent = { + draft: undefined, + spot: mockSpot, + type: 'ENHANCED', + } + + // Enhance same spot twice + handleCommentEvent(message, mockSender) + handleCommentEvent(message, mockSender) + + // Should still be 1 entry (overwritten) + expect(states.size).toBe(1) + }) + }) +}) From b083a7b96709ed4e689848c8e5ecfeca4c63c46a Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 12:28:20 -0700 Subject: [PATCH 09/22] better use of Typescript's type system --- browser-extension/src/entrypoints/content.ts | 1 - browser-extension/src/lib/enhancer.ts | 4 ++-- browser-extension/tests/background-events.test.ts | 11 ++--------- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/browser-extension/src/entrypoints/content.ts b/browser-extension/src/entrypoints/content.ts index f2c1083..83de338 100644 --- a/browser-extension/src/entrypoints/content.ts +++ b/browser-extension/src/entrypoints/content.ts @@ -9,7 +9,6 @@ const enhancedTextareas = new TextareaRegistry() function sendEventToBackground(type: 'ENHANCED' | 'DESTROYED', spot: CommentSpot): void { const message: CommentEvent = { - draft: undefined, spot, type, } diff --git a/browser-extension/src/lib/enhancer.ts b/browser-extension/src/lib/enhancer.ts index c56b78e..a266a7c 100644 --- a/browser-extension/src/lib/enhancer.ts +++ b/browser-extension/src/lib/enhancer.ts @@ -11,7 +11,7 @@ export interface CommentSpot { } export interface CommentDraft { - title: string | undefined + title?: string body: string } @@ -20,7 +20,7 @@ export type CommentEventType = 'ENHANCED' | 'LOST_FOCUS' | 'DESTROYED' export interface CommentEvent { type: CommentEventType spot: CommentSpot - draft: CommentDraft | undefined + draft?: CommentDraft } /** Wraps the textareas of a given platform with Gitcasso's enhancements. */ diff --git a/browser-extension/tests/background-events.test.ts b/browser-extension/tests/background-events.test.ts index a47cf16..5688236 100644 --- a/browser-extension/tests/background-events.test.ts +++ b/browser-extension/tests/background-events.test.ts @@ -26,7 +26,6 @@ describe('Background Event Handler', () => { describe('ENHANCED Event', () => { it('should create new comment state when textarea is enhanced', () => { const message: CommentEvent = { - draft: undefined, spot: mockSpot, type: 'ENHANCED', } @@ -47,7 +46,6 @@ describe('Background Event Handler', () => { it('should not handle ENHANCED event without tab info', () => { const message: CommentEvent = { - draft: undefined, spot: mockSpot, type: 'ENHANCED', } @@ -64,7 +62,6 @@ describe('Background Event Handler', () => { it('should remove comment state when textarea is destroyed', () => { // First create a state using the actual handler const enhanceMessage: CommentEvent = { - draft: undefined, spot: mockSpot, type: 'ENHANCED', } @@ -74,7 +71,6 @@ describe('Background Event Handler', () => { // Then destroy it const destroyMessage: CommentEvent = { - draft: undefined, spot: mockSpot, type: 'DESTROYED', } @@ -86,7 +82,6 @@ describe('Background Event Handler', () => { it('should handle DESTROYED event for non-existent state gracefully', () => { const message: CommentEvent = { - draft: undefined, spot: mockSpot, type: 'DESTROYED', } @@ -101,7 +96,6 @@ describe('Background Event Handler', () => { describe('Invalid Events', () => { it('should ignore events with unsupported type', () => { const message: CommentEvent = { - draft: undefined, spot: mockSpot, type: 'LOST_FOCUS', } @@ -120,15 +114,14 @@ describe('Background Event Handler', () => { const sender1 = { tab: { id: 123, windowId: 456 } } const sender2 = { tab: { id: 789, windowId: 456 } } - handleCommentEvent({ draft: undefined, spot: spot1, type: 'ENHANCED' }, sender1) - handleCommentEvent({ draft: undefined, spot: spot2, type: 'ENHANCED' }, sender2) + handleCommentEvent({ spot: spot1, type: 'ENHANCED' }, sender1) + handleCommentEvent({ spot: spot2, type: 'ENHANCED' }, sender2) expect(states.size).toBe(2) }) it('should handle same spot from same tab (overwrite)', () => { const message: CommentEvent = { - draft: undefined, spot: mockSpot, type: 'ENHANCED', } From 97da1327bc22be08cd444317e36665b3e0c62300 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 12:33:10 -0700 Subject: [PATCH 10/22] tighten up the background test. --- ...ound-events.test.ts => background.test.ts} | 95 ++++++++----------- 1 file changed, 42 insertions(+), 53 deletions(-) rename browser-extension/tests/{background-events.test.ts => background.test.ts} (74%) diff --git a/browser-extension/tests/background-events.test.ts b/browser-extension/tests/background.test.ts similarity index 74% rename from browser-extension/tests/background-events.test.ts rename to browser-extension/tests/background.test.ts index 5688236..333ec9d 100644 --- a/browser-extension/tests/background-events.test.ts +++ b/browser-extension/tests/background.test.ts @@ -2,58 +2,57 @@ import { beforeEach, describe, expect, it } from 'vitest' import { handleCommentEvent, states } from '../src/entrypoints/background' import type { CommentEvent, CommentSpot } from '../src/lib/enhancer' +const mockSender = { + tab: { + id: 123, + windowId: 456, + }, +} +const mockSpot = { + type: 'TEST_SPOT', + unique_key: 'test-key', +} describe('Background Event Handler', () => { - let mockSender: any - let mockSpot: CommentSpot - beforeEach(() => { - // Clear the shared states map before each test states.clear() - - mockSender = { - tab: { - id: 123, - windowId: 456, - }, - } - - mockSpot = { - type: 'TEST_SPOT', - unique_key: 'test-key', - } }) - describe('ENHANCED Event', () => { it('should create new comment state when textarea is enhanced', () => { - const message: CommentEvent = { - spot: mockSpot, - type: 'ENHANCED', - } - - handleCommentEvent(message, mockSender) - - const expectedKey = { - spot: mockSpot, - tab: { tabId: 123, windowId: 456 }, - } - - const state = states.get(expectedKey) - expect(state).toBeDefined() - expect(state?.tab).toEqual({ tabId: 123, windowId: 456 }) - expect(state?.spot).toEqual(mockSpot) - expect(state?.drafts).toEqual([]) + handleCommentEvent( + { + spot: mockSpot, + type: 'ENHANCED', + }, + mockSender, + ) + expect(Array.from(states)).toMatchInlineSnapshot(` + [ + [ + "{"spot":{"type":"TEST_SPOT","unique_key":"test-key"},"tab":{"tabId":123,"windowId":456}}", + { + "drafts": [], + "spot": { + "type": "TEST_SPOT", + "unique_key": "test-key", + }, + "tab": { + "tabId": 123, + "windowId": 456, + }, + }, + ], + ] + `) }) - it('should not handle ENHANCED event without tab info', () => { - const message: CommentEvent = { - spot: mockSpot, - type: 'ENHANCED', - } - const senderWithoutTab = { tab: null } - - handleCommentEvent(message, senderWithoutTab) - + handleCommentEvent( + { + spot: mockSpot, + type: 'ENHANCED', + }, + senderWithoutTab, + ) expect(states.size).toBe(0) }) }) @@ -65,7 +64,6 @@ describe('Background Event Handler', () => { spot: mockSpot, type: 'ENHANCED', } - handleCommentEvent(enhanceMessage, mockSender) expect(states.size).toBe(1) @@ -74,9 +72,7 @@ describe('Background Event Handler', () => { spot: mockSpot, type: 'DESTROYED', } - handleCommentEvent(destroyMessage, mockSender) - expect(states.size).toBe(0) }) @@ -85,10 +81,8 @@ describe('Background Event Handler', () => { spot: mockSpot, type: 'DESTROYED', } - // Should not throw error handleCommentEvent(message, mockSender) - expect(states.size).toBe(0) }) }) @@ -99,9 +93,7 @@ describe('Background Event Handler', () => { spot: mockSpot, type: 'LOST_FOCUS', } - handleCommentEvent(message, mockSender) - expect(states.size).toBe(0) }) }) @@ -110,13 +102,10 @@ describe('Background Event Handler', () => { it('should handle multiple enhanced textareas from different tabs', () => { const spot1: CommentSpot = { type: 'SPOT1', unique_key: 'key1' } const spot2: CommentSpot = { type: 'SPOT2', unique_key: 'key2' } - const sender1 = { tab: { id: 123, windowId: 456 } } const sender2 = { tab: { id: 789, windowId: 456 } } - handleCommentEvent({ spot: spot1, type: 'ENHANCED' }, sender1) handleCommentEvent({ spot: spot2, type: 'ENHANCED' }, sender2) - expect(states.size).toBe(2) }) From d4825129e2e3d80648074901ddf6ea71307a9036 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 14:02:57 -0700 Subject: [PATCH 11/22] Strip lorum ipsum out of the popup. --- .../src/entrypoints/popup/index.html | 9 +------ .../src/entrypoints/popup/main.ts | 26 ------------------- .../src/entrypoints/popup/style.css | 17 ------------ 3 files changed, 1 insertion(+), 51 deletions(-) diff --git a/browser-extension/src/entrypoints/popup/index.html b/browser-extension/src/entrypoints/popup/index.html index a9c17b6..af3b409 100644 --- a/browser-extension/src/entrypoints/popup/index.html +++ b/browser-extension/src/entrypoints/popup/index.html @@ -3,17 +3,10 @@ - Gitcasso Markdown Assistant + Gitcasso
-
- -
Syntax highlighting and autosave for comments on GitHub (and other other markdown-friendly websites).
-
-
-

Loading drafts from local storage...

-

diff --git a/browser-extension/src/entrypoints/popup/main.ts b/browser-extension/src/entrypoints/popup/main.ts index cdb6d2d..cab743a 100644 --- a/browser-extension/src/entrypoints/popup/main.ts +++ b/browser-extension/src/entrypoints/popup/main.ts @@ -1,27 +1 @@ import './style.css' - -document.addEventListener('DOMContentLoaded', async () => { - const statusDiv = document.getElementById('scan-results') as HTMLElement - - try { - // get current active tab - const tabs = await browser.tabs.query({ active: true, currentWindow: true }) - const tab = tabs[0] - - if (!tab?.id) { - statusDiv.textContent = 'Cannot access current tab' - return - } - - // send message to content script to get scan results - const results = await browser.tabs.sendMessage(tab.id, { - action: 'getScanResults', - }) - if (results) { - // TODO: statusDiv.textContent = {{show drafts}} - } - } catch (error) { - console.error('Popup error:', error) - statusDiv.textContent = 'Unable to load saved drafts.' - } -}) diff --git a/browser-extension/src/entrypoints/popup/style.css b/browser-extension/src/entrypoints/popup/style.css index 40c75e1..3407a18 100644 --- a/browser-extension/src/entrypoints/popup/style.css +++ b/browser-extension/src/entrypoints/popup/style.css @@ -6,20 +6,3 @@ body { line-height: 1.4; margin: 0; } - -.header { - text-align: center; - margin-bottom: 15px; -} - -.logo { - font-size: 18px; - font-weight: bold; - color: #0066cc; - margin-bottom: 8px; -} - -.subtitle { - color: #666; - font-size: 12px; -} From 8befd38a88169235dab24ccb6a48d468766f154e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 14:03:10 -0700 Subject: [PATCH 12/22] Rename `states` to `openSpots`. --- .../src/entrypoints/background.ts | 6 +++--- browser-extension/tests/background.test.ts | 20 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/browser-extension/src/entrypoints/background.ts b/browser-extension/src/entrypoints/background.ts index ac15743..66e6752 100644 --- a/browser-extension/src/entrypoints/background.ts +++ b/browser-extension/src/entrypoints/background.ts @@ -15,7 +15,7 @@ export interface CommentState { drafts: [number, CommentDraft][] } -export const states = new JsonMap() +export const openSpots = new JsonMap() export function handleCommentEvent(message: CommentEvent, sender: any): void { if ( @@ -39,9 +39,9 @@ export function handleCommentEvent(message: CommentEvent, sender: any): void { spot: message.spot, tab, } - states.set(tabAndSpot, commentState) + openSpots.set(tabAndSpot, commentState) } else if (message.type === 'DESTROYED') { - states.delete(tabAndSpot) + openSpots.delete(tabAndSpot) } } } diff --git a/browser-extension/tests/background.test.ts b/browser-extension/tests/background.test.ts index 333ec9d..988ceff 100644 --- a/browser-extension/tests/background.test.ts +++ b/browser-extension/tests/background.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it } from 'vitest' -import { handleCommentEvent, states } from '../src/entrypoints/background' +import { handleCommentEvent, openSpots } from '../src/entrypoints/background' import type { CommentEvent, CommentSpot } from '../src/lib/enhancer' const mockSender = { @@ -14,7 +14,7 @@ const mockSpot = { } describe('Background Event Handler', () => { beforeEach(() => { - states.clear() + openSpots.clear() }) describe('ENHANCED Event', () => { it('should create new comment state when textarea is enhanced', () => { @@ -25,7 +25,7 @@ describe('Background Event Handler', () => { }, mockSender, ) - expect(Array.from(states)).toMatchInlineSnapshot(` + expect(Array.from(openSpots)).toMatchInlineSnapshot(` [ [ "{"spot":{"type":"TEST_SPOT","unique_key":"test-key"},"tab":{"tabId":123,"windowId":456}}", @@ -53,7 +53,7 @@ describe('Background Event Handler', () => { }, senderWithoutTab, ) - expect(states.size).toBe(0) + expect(openSpots.size).toBe(0) }) }) @@ -65,7 +65,7 @@ describe('Background Event Handler', () => { type: 'ENHANCED', } handleCommentEvent(enhanceMessage, mockSender) - expect(states.size).toBe(1) + expect(openSpots.size).toBe(1) // Then destroy it const destroyMessage: CommentEvent = { @@ -73,7 +73,7 @@ describe('Background Event Handler', () => { type: 'DESTROYED', } handleCommentEvent(destroyMessage, mockSender) - expect(states.size).toBe(0) + expect(openSpots.size).toBe(0) }) it('should handle DESTROYED event for non-existent state gracefully', () => { @@ -83,7 +83,7 @@ describe('Background Event Handler', () => { } // Should not throw error handleCommentEvent(message, mockSender) - expect(states.size).toBe(0) + expect(openSpots.size).toBe(0) }) }) @@ -94,7 +94,7 @@ describe('Background Event Handler', () => { type: 'LOST_FOCUS', } handleCommentEvent(message, mockSender) - expect(states.size).toBe(0) + expect(openSpots.size).toBe(0) }) }) @@ -106,7 +106,7 @@ describe('Background Event Handler', () => { const sender2 = { tab: { id: 789, windowId: 456 } } handleCommentEvent({ spot: spot1, type: 'ENHANCED' }, sender1) handleCommentEvent({ spot: spot2, type: 'ENHANCED' }, sender2) - expect(states.size).toBe(2) + expect(openSpots.size).toBe(2) }) it('should handle same spot from same tab (overwrite)', () => { @@ -120,7 +120,7 @@ describe('Background Event Handler', () => { handleCommentEvent(message, mockSender) // Should still be 1 entry (overwritten) - expect(states.size).toBe(1) + expect(openSpots.size).toBe(1) }) }) }) From 5f0eac810f8376776c799f52e34ef0dbd5c2cacf Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 14:13:13 -0700 Subject: [PATCH 13/22] first cut at popup implementation --- .../src/entrypoints/background.ts | 20 +++++- .../src/entrypoints/popup/main.ts | 62 +++++++++++++++++++ .../src/entrypoints/popup/style.css | 45 ++++++++++++++ 3 files changed, 126 insertions(+), 1 deletion(-) diff --git a/browser-extension/src/entrypoints/background.ts b/browser-extension/src/entrypoints/background.ts index 66e6752..88cc665 100644 --- a/browser-extension/src/entrypoints/background.ts +++ b/browser-extension/src/entrypoints/background.ts @@ -46,6 +46,24 @@ export function handleCommentEvent(message: CommentEvent, sender: any): void { } } +export function handlePopupMessage(message: any, sender: any, sendResponse: (response: any) => void): void { + if (message.type === 'GET_OPEN_SPOTS') { + const spots: CommentState[] = [] + for (const [, commentState] of openSpots) { + spots.push(commentState) + } + sendResponse({ spots }) + } +} + export default defineBackground(() => { - browser.runtime.onMessage.addListener(handleCommentEvent) + browser.runtime.onMessage.addListener((message, sender, sendResponse) => { + if (message.type === 'GET_OPEN_SPOTS') { + handlePopupMessage(message, sender, sendResponse) + return true + } else { + handleCommentEvent(message, sender) + return false + } + }) }) diff --git a/browser-extension/src/entrypoints/popup/main.ts b/browser-extension/src/entrypoints/popup/main.ts index cab743a..6493cc3 100644 --- a/browser-extension/src/entrypoints/popup/main.ts +++ b/browser-extension/src/entrypoints/popup/main.ts @@ -1 +1,63 @@ import './style.css' +import type { CommentState } from '../background' +import { EnhancerRegistry } from '../../lib/registries' + +const enhancers = new EnhancerRegistry() + +async function getOpenSpots(): Promise { + return new Promise((resolve) => { + browser.runtime.sendMessage({ type: 'GET_OPEN_SPOTS' }, (response) => { + resolve(response.spots || []) + }) + }) +} + +async function switchToTab(tabId: number, windowId: number): Promise { + await browser.windows.update(windowId, { focused: true }) + await browser.tabs.update(tabId, { active: true }) + window.close() +} + +function createSpotElement(commentState: CommentState): HTMLElement { + const item = document.createElement('div') + item.className = 'spot-item' + + const title = document.createElement('div') + title.className = 'spot-title' + + const enhancer = enhancers.enhancerFor(commentState.spot) + title.textContent = enhancer.tableTitle(commentState.spot) + + item.appendChild(title) + + item.addEventListener('click', () => { + switchToTab(commentState.tab.tabId, commentState.tab.windowId) + }) + + return item +} + +async function renderOpenSpots(): Promise { + const app = document.getElementById('app')! + const spots = await getOpenSpots() + + if (spots.length === 0) { + app.innerHTML = '
No open comment spots
' + return + } + + const header = document.createElement('h2') + header.textContent = 'Open Comment Spots' + app.appendChild(header) + + const list = document.createElement('div') + list.className = 'spots-list' + + spots.forEach(spot => { + list.appendChild(createSpotElement(spot)) + }) + + app.appendChild(list) +} + +renderOpenSpots() diff --git a/browser-extension/src/entrypoints/popup/style.css b/browser-extension/src/entrypoints/popup/style.css index 3407a18..0d63390 100644 --- a/browser-extension/src/entrypoints/popup/style.css +++ b/browser-extension/src/entrypoints/popup/style.css @@ -6,3 +6,48 @@ body { line-height: 1.4; margin: 0; } + +h2 { + margin: 0 0 15px 0; + font-size: 16px; + font-weight: 600; + color: #333; +} + +.spots-list { + display: flex; + flex-direction: column; + gap: 8px; +} + +.spot-item { + padding: 10px; + border: 1px solid #e0e0e0; + border-radius: 6px; + cursor: pointer; + transition: all 0.2s ease; + background: white; +} + +.spot-item:hover { + background: #f5f5f5; + border-color: #d0d0d0; + transform: translateY(-1px); + box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1); +} + +.spot-title { + font-weight: 500; + color: #333; + margin-bottom: 4px; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.no-spots { + text-align: center; + color: #666; + padding: 40px 20px; + font-style: italic; +} From 762725356d3deaa71c2c7c38767353d799d77e67 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 14:18:07 -0700 Subject: [PATCH 14/22] Add a bit of logging. --- browser-extension/src/entrypoints/popup/main.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/browser-extension/src/entrypoints/popup/main.ts b/browser-extension/src/entrypoints/popup/main.ts index 6493cc3..cc6227b 100644 --- a/browser-extension/src/entrypoints/popup/main.ts +++ b/browser-extension/src/entrypoints/popup/main.ts @@ -1,6 +1,7 @@ import './style.css' import type { CommentState } from '../background' import { EnhancerRegistry } from '../../lib/registries' +import { logger } from '../../lib/logger' const enhancers = new EnhancerRegistry() @@ -38,8 +39,11 @@ function createSpotElement(commentState: CommentState): HTMLElement { } async function renderOpenSpots(): Promise { + logger.debug('renderOpenSpots') const app = document.getElementById('app')! + logger.debug('waiting on getOpenSpots') const spots = await getOpenSpots() + logger.debug('got', spots) if (spots.length === 0) { app.innerHTML = '
No open comment spots
' From 25b46a71208088cba63c78488ff948112de3e32c Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 14:29:57 -0700 Subject: [PATCH 15/22] This has plenty of problems, but it does turn on! --- .../src/entrypoints/popup/main.ts | 47 ++++++++++++++----- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/browser-extension/src/entrypoints/popup/main.ts b/browser-extension/src/entrypoints/popup/main.ts index cc6227b..7192925 100644 --- a/browser-extension/src/entrypoints/popup/main.ts +++ b/browser-extension/src/entrypoints/popup/main.ts @@ -3,14 +3,29 @@ import type { CommentState } from '../background' import { EnhancerRegistry } from '../../lib/registries' import { logger } from '../../lib/logger' +// Test basic DOM access +try { + const app = document.getElementById('app') + logger.debug('Found app element:', app) + if (app) { + app.innerHTML = '
Script is running...
' + } +} catch (error) { + logger.error('Error accessing DOM:', error) +} + const enhancers = new EnhancerRegistry() async function getOpenSpots(): Promise { - return new Promise((resolve) => { - browser.runtime.sendMessage({ type: 'GET_OPEN_SPOTS' }, (response) => { - resolve(response.spots || []) - }) - }) + logger.debug('Sending message to background script...') + try { + const response = await browser.runtime.sendMessage({ type: 'GET_OPEN_SPOTS' }) + logger.debug('Received response:', response) + return response.spots || [] + } catch (error) { + logger.error('Error sending message to background:', error) + return [] + } } async function switchToTab(tabId: number, windowId: number): Promise { @@ -23,11 +38,18 @@ function createSpotElement(commentState: CommentState): HTMLElement { const item = document.createElement('div') item.className = 'spot-item' + logger.debug('Creating spot element for:', commentState.spot) + const enhancer = enhancers.enhancerFor(commentState.spot) + logger.debug('Found enhancer:', enhancer) + const title = document.createElement('div') title.className = 'spot-title' - const enhancer = enhancers.enhancerFor(commentState.spot) - title.textContent = enhancer.tableTitle(commentState.spot) + if (enhancer) { + title.textContent = enhancer.tableTitle(commentState.spot) + } else { + title.textContent = `${commentState.spot.type} (${commentState.spot.unique_key})` + } item.appendChild(title) @@ -39,11 +61,10 @@ function createSpotElement(commentState: CommentState): HTMLElement { } async function renderOpenSpots(): Promise { - logger.debug('renderOpenSpots') + logger.debug('renderOpenSpots called') const app = document.getElementById('app')! - logger.debug('waiting on getOpenSpots') const spots = await getOpenSpots() - logger.debug('got', spots) + logger.debug('Got spots:', spots) if (spots.length === 0) { app.innerHTML = '
No open comment spots
' @@ -64,4 +85,8 @@ async function renderOpenSpots(): Promise { app.appendChild(list) } -renderOpenSpots() +renderOpenSpots().catch(error => { + logger.error('Error in renderOpenSpots:', error) + const app = document.getElementById('app')! + app.innerHTML = `
Error loading spots: ${error.message}
` +}) From f3734c651b0a3a2aca87cda374c4b77264b6df21 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 15:28:14 -0700 Subject: [PATCH 16/22] working at a basic level --- .../src/entrypoints/background.ts | 10 ++++-- .../src/entrypoints/popup/main.ts | 32 +++++++++---------- browser-extension/src/lib/enhancer.ts | 1 - browser-extension/src/lib/registries.ts | 4 +-- browser-extension/wxt.config.ts | 2 +- 5 files changed, 26 insertions(+), 23 deletions(-) diff --git a/browser-extension/src/entrypoints/background.ts b/browser-extension/src/entrypoints/background.ts index 88cc665..91a9b20 100644 --- a/browser-extension/src/entrypoints/background.ts +++ b/browser-extension/src/entrypoints/background.ts @@ -46,19 +46,25 @@ export function handleCommentEvent(message: CommentEvent, sender: any): void { } } -export function handlePopupMessage(message: any, sender: any, sendResponse: (response: any) => void): void { +export function handlePopupMessage(message: any, _sender: any, sendResponse: (response: any) => void): void { if (message.type === 'GET_OPEN_SPOTS') { const spots: CommentState[] = [] for (const [, commentState] of openSpots) { spots.push(commentState) } sendResponse({ spots }) + } else if (message.type === 'SWITCH_TO_TAB') { + browser.windows.update(message.windowId, { focused: true }).then(() => { + return browser.tabs.update(message.tabId, { active: true }) + }).catch(error => { + console.error('Error switching to tab:', error) + }) } } export default defineBackground(() => { browser.runtime.onMessage.addListener((message, sender, sendResponse) => { - if (message.type === 'GET_OPEN_SPOTS') { + if (message.type === 'GET_OPEN_SPOTS' || message.type === 'SWITCH_TO_TAB') { handlePopupMessage(message, sender, sendResponse) return true } else { diff --git a/browser-extension/src/entrypoints/popup/main.ts b/browser-extension/src/entrypoints/popup/main.ts index 7192925..7527ca3 100644 --- a/browser-extension/src/entrypoints/popup/main.ts +++ b/browser-extension/src/entrypoints/popup/main.ts @@ -5,11 +5,9 @@ import { logger } from '../../lib/logger' // Test basic DOM access try { - const app = document.getElementById('app') + const app = document.getElementById('app')!! logger.debug('Found app element:', app) - if (app) { - app.innerHTML = '
Script is running...
' - } + app.innerHTML = '
Script is running...
' } catch (error) { logger.error('Error accessing DOM:', error) } @@ -28,9 +26,14 @@ async function getOpenSpots(): Promise { } } -async function switchToTab(tabId: number, windowId: number): Promise { - await browser.windows.update(windowId, { focused: true }) - await browser.tabs.update(tabId, { active: true }) +function switchToTab(tabId: number, windowId: number): void { + // Send message to background script to handle tab switching + // This avoids the popup context being destroyed before completion + browser.runtime.sendMessage({ + type: 'SWITCH_TO_TAB', + tabId, + windowId + }) window.close() } @@ -40,23 +43,18 @@ function createSpotElement(commentState: CommentState): HTMLElement { logger.debug('Creating spot element for:', commentState.spot) const enhancer = enhancers.enhancerFor(commentState.spot) - logger.debug('Found enhancer:', enhancer) + if (!enhancer) { + logger.error('No enhancer found for:', commentState.spot) + logger.error('Only have enhancers for:', enhancers.byType) + } const title = document.createElement('div') title.className = 'spot-title' - - if (enhancer) { - title.textContent = enhancer.tableTitle(commentState.spot) - } else { - title.textContent = `${commentState.spot.type} (${commentState.spot.unique_key})` - } - + title.textContent = enhancer.tableTitle(commentState.spot) item.appendChild(title) - item.addEventListener('click', () => { switchToTab(commentState.tab.tabId, commentState.tab.windowId) }) - return item } diff --git a/browser-extension/src/lib/enhancer.ts b/browser-extension/src/lib/enhancer.ts index a266a7c..0895454 100644 --- a/browser-extension/src/lib/enhancer.ts +++ b/browser-extension/src/lib/enhancer.ts @@ -43,5 +43,4 @@ export interface CommentEnhancer { tableIcon(spot: Spot): string tableTitle(spot: Spot): string - buildUrl(spot: Spot): string } diff --git a/browser-extension/src/lib/registries.ts b/browser-extension/src/lib/registries.ts index 79fdd2f..b6d8ad9 100644 --- a/browser-extension/src/lib/registries.ts +++ b/browser-extension/src/lib/registries.ts @@ -13,7 +13,7 @@ export interface EnhancedTextarea { export class EnhancerRegistry { private enhancers = new Set() private preparedEnhancers = new Set() - private byType = new Map() + byType = new Map() constructor() { // Register all available handlers @@ -23,7 +23,7 @@ export class EnhancerRegistry { private register(enhancer: CommentEnhancer): void { this.enhancers.add(enhancer) - for (const spotType in enhancer.forSpotTypes()) { + for (const spotType of enhancer.forSpotTypes()) { this.byType.set(spotType, enhancer) } } diff --git a/browser-extension/wxt.config.ts b/browser-extension/wxt.config.ts index edc2589..7a4699a 100644 --- a/browser-extension/wxt.config.ts +++ b/browser-extension/wxt.config.ts @@ -11,7 +11,7 @@ export default defineConfig({ 128: '/icons/icon-128.png', }, name: 'Gitcasso', - permissions: ['activeTab'], + permissions: ['activeTab', 'tabs'], version: '1.0.0', }, modules: ['@wxt-dev/webextension-polyfill'], From 23348958e75871e67f90792dd0a998b3211108fc Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 16:43:48 -0700 Subject: [PATCH 17/22] Leverage the typesystem. --- .../src/entrypoints/background.ts | 42 ++++++---- .../src/entrypoints/popup/main.ts | 27 ++++--- browser-extension/src/lib/messages.ts | 76 +++++++++++++++++++ 3 files changed, 121 insertions(+), 24 deletions(-) create mode 100644 browser-extension/src/lib/messages.ts diff --git a/browser-extension/src/entrypoints/background.ts b/browser-extension/src/entrypoints/background.ts index 91a9b20..d158bf8 100644 --- a/browser-extension/src/entrypoints/background.ts +++ b/browser-extension/src/entrypoints/background.ts @@ -1,5 +1,11 @@ import type { CommentDraft, CommentEvent, CommentSpot } from '../lib/enhancer' import { JsonMap } from '../lib/jsonmap' +import type { GetOpenSpotsResponse, ToBackgroundMessage } from '../lib/messages' +import { + isContentToBackgroundMessage, + isGetOpenSpotsMessage, + isSwitchToTabMessage, +} from '../lib/messages' export interface Tab { tabId: number @@ -46,30 +52,38 @@ export function handleCommentEvent(message: CommentEvent, sender: any): void { } } -export function handlePopupMessage(message: any, _sender: any, sendResponse: (response: any) => void): void { - if (message.type === 'GET_OPEN_SPOTS') { +export function handlePopupMessage( + message: any, + _sender: any, + sendResponse: (response: any) => void, +): void { + if (isGetOpenSpotsMessage(message)) { const spots: CommentState[] = [] for (const [, commentState] of openSpots) { spots.push(commentState) } - sendResponse({ spots }) - } else if (message.type === 'SWITCH_TO_TAB') { - browser.windows.update(message.windowId, { focused: true }).then(() => { - return browser.tabs.update(message.tabId, { active: true }) - }).catch(error => { - console.error('Error switching to tab:', error) - }) + const response: GetOpenSpotsResponse = { spots } + sendResponse(response) + } else if (isSwitchToTabMessage(message)) { + browser.windows + .update(message.windowId, { focused: true }) + .then(() => { + return browser.tabs.update(message.tabId, { active: true }) + }) + .catch((error) => { + console.error('Error switching to tab:', error) + }) } } export default defineBackground(() => { - browser.runtime.onMessage.addListener((message, sender, sendResponse) => { - if (message.type === 'GET_OPEN_SPOTS' || message.type === 'SWITCH_TO_TAB') { - handlePopupMessage(message, sender, sendResponse) - return true - } else { + browser.runtime.onMessage.addListener((message: ToBackgroundMessage, sender, sendResponse) => { + if (isContentToBackgroundMessage(message)) { handleCommentEvent(message, sender) return false + } else { + handlePopupMessage(message, sender, sendResponse) + return true } }) }) diff --git a/browser-extension/src/entrypoints/popup/main.ts b/browser-extension/src/entrypoints/popup/main.ts index 7527ca3..4ccea9a 100644 --- a/browser-extension/src/entrypoints/popup/main.ts +++ b/browser-extension/src/entrypoints/popup/main.ts @@ -1,11 +1,16 @@ import './style.css' -import type { CommentState } from '../background' -import { EnhancerRegistry } from '../../lib/registries' import { logger } from '../../lib/logger' +import type { + GetOpenSpotsMessage, + GetOpenSpotsResponse, + SwitchToTabMessage, +} from '../../lib/messages' +import { EnhancerRegistry } from '../../lib/registries' +import type { CommentState } from '../background' // Test basic DOM access try { - const app = document.getElementById('app')!! + const app = document.getElementById('app')! logger.debug('Found app element:', app) app.innerHTML = '
Script is running...
' } catch (error) { @@ -17,7 +22,8 @@ const enhancers = new EnhancerRegistry() async function getOpenSpots(): Promise { logger.debug('Sending message to background script...') try { - const response = await browser.runtime.sendMessage({ type: 'GET_OPEN_SPOTS' }) + const message: GetOpenSpotsMessage = { type: 'GET_OPEN_SPOTS' } + const response = (await browser.runtime.sendMessage(message)) as GetOpenSpotsResponse logger.debug('Received response:', response) return response.spots || [] } catch (error) { @@ -29,11 +35,12 @@ async function getOpenSpots(): Promise { function switchToTab(tabId: number, windowId: number): void { // Send message to background script to handle tab switching // This avoids the popup context being destroyed before completion - browser.runtime.sendMessage({ - type: 'SWITCH_TO_TAB', + const message: SwitchToTabMessage = { tabId, - windowId - }) + type: 'SWITCH_TO_TAB', + windowId, + } + browser.runtime.sendMessage(message) window.close() } @@ -76,14 +83,14 @@ async function renderOpenSpots(): Promise { const list = document.createElement('div') list.className = 'spots-list' - spots.forEach(spot => { + spots.forEach((spot) => { list.appendChild(createSpotElement(spot)) }) app.appendChild(list) } -renderOpenSpots().catch(error => { +renderOpenSpots().catch((error) => { logger.error('Error in renderOpenSpots:', error) const app = document.getElementById('app')! app.innerHTML = `
Error loading spots: ${error.message}
` diff --git a/browser-extension/src/lib/messages.ts b/browser-extension/src/lib/messages.ts new file mode 100644 index 0000000..127d921 --- /dev/null +++ b/browser-extension/src/lib/messages.ts @@ -0,0 +1,76 @@ +import type { CommentDraft, CommentEvent, CommentSpot } from './enhancer' + +// Content -> Background messages (already well-typed as CommentEvent) +export type ContentToBackgroundMessage = CommentEvent + +// Popup -> Background messages +export interface GetOpenSpotsMessage { + type: 'GET_OPEN_SPOTS' +} + +export interface SwitchToTabMessage { + type: 'SWITCH_TO_TAB' + tabId: number + windowId: number +} + +export type PopupToBackgroundMessage = GetOpenSpotsMessage | SwitchToTabMessage + +// All messages sent to background +export type ToBackgroundMessage = ContentToBackgroundMessage | PopupToBackgroundMessage + +// Background -> Popup responses +export interface GetOpenSpotsResponse { + spots: Array<{ + tab: { + tabId: number + windowId: number + } + spot: CommentSpot + drafts: Array<[number, CommentDraft]> + }> +} + +// Type guard functions +export function isContentToBackgroundMessage(message: any): message is ContentToBackgroundMessage { + return ( + message && + typeof message.type === 'string' && + (message.type === 'ENHANCED' || message.type === 'DESTROYED') && + message.spot + ) +} + +export function isPopupToBackgroundMessage(message: any): message is PopupToBackgroundMessage { + return ( + message && + typeof message.type === 'string' && + (message.type === 'GET_OPEN_SPOTS' || message.type === 'SWITCH_TO_TAB') + ) +} + +export function isGetOpenSpotsMessage(message: any): message is GetOpenSpotsMessage { + return message && message.type === 'GET_OPEN_SPOTS' +} + +export function isSwitchToTabMessage(message: any): message is SwitchToTabMessage { + return ( + message && + message.type === 'SWITCH_TO_TAB' && + typeof message.tabId === 'number' && + typeof message.windowId === 'number' + ) +} + +// Message handler types +export type BackgroundMessageHandler = ( + message: ToBackgroundMessage, + sender: any, + sendResponse: (response?: any) => void, +) => boolean | undefined + +export type PopupMessageSender = { + sendMessage( + message: T, + ): Promise +} From 8bdf3b8b0b21dc6ca7cabc5ac065aa1103b6b85b Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 16:57:16 -0700 Subject: [PATCH 18/22] Add types for the flag which determines if a response is going to be sent or not. --- .../src/entrypoints/background.ts | 21 ++++++++++++------- browser-extension/src/lib/messages.ts | 4 ++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/browser-extension/src/entrypoints/background.ts b/browser-extension/src/entrypoints/background.ts index d158bf8..e1e19bf 100644 --- a/browser-extension/src/entrypoints/background.ts +++ b/browser-extension/src/entrypoints/background.ts @@ -5,6 +5,8 @@ import { isContentToBackgroundMessage, isGetOpenSpotsMessage, isSwitchToTabMessage, + CLOSE_MESSAGE_PORT, + KEEP_PORT_OPEN, } from '../lib/messages' export interface Tab { @@ -23,7 +25,7 @@ export interface CommentState { export const openSpots = new JsonMap() -export function handleCommentEvent(message: CommentEvent, sender: any): void { +export function handleCommentEvent(message: CommentEvent, sender: any): boolean { if ( (message.type === 'ENHANCED' || message.type === 'DESTROYED') && sender.tab?.id && @@ -33,12 +35,10 @@ export function handleCommentEvent(message: CommentEvent, sender: any): void { tabId: sender.tab.id, windowId: sender.tab.windowId, } - const tabAndSpot: TabAndSpot = { spot: message.spot, tab, } - if (message.type === 'ENHANCED') { const commentState: CommentState = { drafts: [], @@ -48,15 +48,18 @@ export function handleCommentEvent(message: CommentEvent, sender: any): void { openSpots.set(tabAndSpot, commentState) } else if (message.type === 'DESTROYED') { openSpots.delete(tabAndSpot) + } else { + throw new Error(`Unhandled comment event type: ${message.type}`) } } + return CLOSE_MESSAGE_PORT } export function handlePopupMessage( message: any, _sender: any, sendResponse: (response: any) => void, -): void { +): typeof CLOSE_MESSAGE_PORT | typeof KEEP_PORT_OPEN { if (isGetOpenSpotsMessage(message)) { const spots: CommentState[] = [] for (const [, commentState] of openSpots) { @@ -64,6 +67,7 @@ export function handlePopupMessage( } const response: GetOpenSpotsResponse = { spots } sendResponse(response) + return KEEP_PORT_OPEN } else if (isSwitchToTabMessage(message)) { browser.windows .update(message.windowId, { focused: true }) @@ -73,17 +77,18 @@ export function handlePopupMessage( .catch((error) => { console.error('Error switching to tab:', error) }) + return CLOSE_MESSAGE_PORT + } else { + throw new Error(`Unhandled popup message type: ${message?.type || 'unknown'}`) } } export default defineBackground(() => { browser.runtime.onMessage.addListener((message: ToBackgroundMessage, sender, sendResponse) => { if (isContentToBackgroundMessage(message)) { - handleCommentEvent(message, sender) - return false + return handleCommentEvent(message, sender) } else { - handlePopupMessage(message, sender, sendResponse) - return true + return handlePopupMessage(message, sender, sendResponse) } }) }) diff --git a/browser-extension/src/lib/messages.ts b/browser-extension/src/lib/messages.ts index 127d921..e04e8c5 100644 --- a/browser-extension/src/lib/messages.ts +++ b/browser-extension/src/lib/messages.ts @@ -1,5 +1,9 @@ import type { CommentDraft, CommentEvent, CommentSpot } from './enhancer' +// Message handler response types +export const CLOSE_MESSAGE_PORT = false as const // No response will be sent +export const KEEP_PORT_OPEN = true as const // Response will be sent (possibly async) + // Content -> Background messages (already well-typed as CommentEvent) export type ContentToBackgroundMessage = CommentEvent From 15adc2eac55ffc2b6f7c65af507ef175a76c9099 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 17:02:10 -0700 Subject: [PATCH 19/22] Add a handy precommit script to `browser-extension`. --- browser-extension/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/browser-extension/package.json b/browser-extension/package.json index 7a6b835..128aa79 100644 --- a/browser-extension/package.json +++ b/browser-extension/package.json @@ -41,6 +41,7 @@ "build": "wxt build", "build:dev": "wxt build --mode development", "build:firefox": "wxt build -b firefox", + "precommit": "npm run biome:fix && npm run typecheck && npm run test", "typecheck": "tsc --noEmit", "dev": "wxt", "dev:firefox": "wxt -b firefox", From b020899d4cdc930d7cab2d430406824dff1b29fe Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 17:40:43 -0700 Subject: [PATCH 20/22] Oops, forgot a sort. --- browser-extension/src/entrypoints/background.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser-extension/src/entrypoints/background.ts b/browser-extension/src/entrypoints/background.ts index e1e19bf..988c338 100644 --- a/browser-extension/src/entrypoints/background.ts +++ b/browser-extension/src/entrypoints/background.ts @@ -2,10 +2,10 @@ import type { CommentDraft, CommentEvent, CommentSpot } from '../lib/enhancer' import { JsonMap } from '../lib/jsonmap' import type { GetOpenSpotsResponse, ToBackgroundMessage } from '../lib/messages' import { + CLOSE_MESSAGE_PORT, isContentToBackgroundMessage, isGetOpenSpotsMessage, isSwitchToTabMessage, - CLOSE_MESSAGE_PORT, KEEP_PORT_OPEN, } from '../lib/messages' From e8ca1d68270ab44020368b4a98f0bb8273fedf8f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 17:50:18 -0700 Subject: [PATCH 21/22] Add a mermaid diagram for the new layout. --- browser-extension/README.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/browser-extension/README.md b/browser-extension/README.md index 0070352..cb2641f 100644 --- a/browser-extension/README.md +++ b/browser-extension/README.md @@ -34,8 +34,32 @@ This is a [WXT](https://wxt.dev/)-based browser extension that ### Entry points - `src/entrypoints/content.ts` - injected into every webpage +- `src/entrypoints/background.ts` - service worker that manages state and handles messages - `src/entrypoints/popup` - html/css/ts which opens when the extension's button gets clicked +```mermaid +graph TD + Content[Content Script
content.ts] + Background[Background Script
background.ts] + Popup[Popup Script
popup/main.ts] + + Content -->|ENHANCED/DESTROYED
CommentEvent| Background + Popup -->|GET_OPEN_SPOTS
SWITCH_TO_TAB| Background + Background -->|GetOpenSpotsResponse
spots array| Popup + + Background -.->|manages| Storage[Comment State Storage
openSpots JsonMap] + Content -.->|enhances| TextArea[textarea elements
on web pages] + Popup -.->|displays| UI[Extension UI
list of comment spots] + + classDef entrypoint fill:#e1f5fe + classDef storage fill:#f3e5f5 + classDef ui fill:#e8f5e8 + + class Content,Background,Popup entrypoint + class Storage storage + class TextArea,UI ui +``` + ### Architecture Every time a `textarea` shows up on a page, on initial load or later on, it gets passed to a list of `CommentEnhancer`s. Each one gets a turn to say "I can enhance this box!". They show that they can enhance it by returning a [`CommentSpot`, `Overtype`]. From a8fffaa2c167caccc9061865dee4c0cc76c72db6 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 9 Sep 2025 17:53:19 -0700 Subject: [PATCH 22/22] We can simplify by keying on `unique_key` like we're supposed to, lol --- .../src/entrypoints/background.ts | 23 +++------ browser-extension/src/lib/jsonmap.ts | 51 ------------------- browser-extension/tests/background.test.ts | 2 +- 3 files changed, 8 insertions(+), 68 deletions(-) delete mode 100644 browser-extension/src/lib/jsonmap.ts diff --git a/browser-extension/src/entrypoints/background.ts b/browser-extension/src/entrypoints/background.ts index 988c338..3e071a1 100644 --- a/browser-extension/src/entrypoints/background.ts +++ b/browser-extension/src/entrypoints/background.ts @@ -1,5 +1,4 @@ import type { CommentDraft, CommentEvent, CommentSpot } from '../lib/enhancer' -import { JsonMap } from '../lib/jsonmap' import type { GetOpenSpotsResponse, ToBackgroundMessage } from '../lib/messages' import { CLOSE_MESSAGE_PORT, @@ -13,17 +12,13 @@ export interface Tab { tabId: number windowId: number } -export interface TabAndSpot { - tab: Tab - spot: CommentSpot -} export interface CommentState { tab: Tab spot: CommentSpot drafts: [number, CommentDraft][] } -export const openSpots = new JsonMap() +export const openSpots = new Map() export function handleCommentEvent(message: CommentEvent, sender: any): boolean { if ( @@ -31,23 +26,19 @@ export function handleCommentEvent(message: CommentEvent, sender: any): boolean sender.tab?.id && sender.tab?.windowId ) { - const tab: Tab = { - tabId: sender.tab.id, - windowId: sender.tab.windowId, - } - const tabAndSpot: TabAndSpot = { - spot: message.spot, - tab, - } if (message.type === 'ENHANCED') { + const tab: Tab = { + tabId: sender.tab.id, + windowId: sender.tab.windowId, + } const commentState: CommentState = { drafts: [], spot: message.spot, tab, } - openSpots.set(tabAndSpot, commentState) + openSpots.set(message.spot.unique_key, commentState) } else if (message.type === 'DESTROYED') { - openSpots.delete(tabAndSpot) + openSpots.delete(message.spot.unique_key) } else { throw new Error(`Unhandled comment event type: ${message.type}`) } diff --git a/browser-extension/src/lib/jsonmap.ts b/browser-extension/src/lib/jsonmap.ts deleted file mode 100644 index aa26675..0000000 --- a/browser-extension/src/lib/jsonmap.ts +++ /dev/null @@ -1,51 +0,0 @@ -/** Map which can take any JSON as key and uses its sorted serialization as the underlying key. */ -export class JsonMap { - private map = new Map() - - set(key: K, value: V): this { - this.map.set(stableStringify(key), value) - return this - } - - get(key: K): V | undefined { - return this.map.get(stableStringify(key)) - } - - has(key: K): boolean { - return this.map.has(stableStringify(key)) - } - - delete(key: K): boolean { - return this.map.delete(stableStringify(key)) - } - - clear(): void { - this.map.clear() - } - - get size(): number { - return this.map.size - } - - [Symbol.iterator](): IterableIterator<[string, V]> { - return this.map.entries() - } -} - -function stableStringify(v: unknown): string { - return JSON.stringify(v, (_k, val) => { - if (val && typeof val === 'object' && !Array.isArray(val)) { - // sort object keys recursively - return Object.keys(val as Record) - .sort() - .reduce( - (acc, k) => { - ;(acc as any)[k] = (val as any)[k] - return acc - }, - {} as Record, - ) - } - return val - }) -} diff --git a/browser-extension/tests/background.test.ts b/browser-extension/tests/background.test.ts index 988ceff..961e1d0 100644 --- a/browser-extension/tests/background.test.ts +++ b/browser-extension/tests/background.test.ts @@ -28,7 +28,7 @@ describe('Background Event Handler', () => { expect(Array.from(openSpots)).toMatchInlineSnapshot(` [ [ - "{"spot":{"type":"TEST_SPOT","unique_key":"test-key"},"tab":{"tabId":123,"windowId":456}}", + "test-key", { "drafts": [], "spot": {