diff --git a/.changeset/refactor-close-handler-and-settings.md b/.changeset/refactor-close-handler-and-settings.md new file mode 100644 index 0000000..8f277f0 --- /dev/null +++ b/.changeset/refactor-close-handler-and-settings.md @@ -0,0 +1,5 @@ +--- +'elden-ring-github': patch +--- + +Refactor close detection module and consolidate settings management. Renamed `closeWatcher` to `closeHandler` for naming consistency and merged `SettingsStore` into `ShowSettings` to reduce unnecessary abstraction. diff --git a/src/content/closeWatcher.test.ts b/src/content/closeHandler.test.ts similarity index 86% rename from src/content/closeWatcher.test.ts rename to src/content/closeHandler.test.ts index 3a02326..bdda36d 100644 --- a/src/content/closeWatcher.test.ts +++ b/src/content/closeHandler.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { waitForCloseComplete } from './closeWatcher'; +import { waitForCloseCompletion } from './closeHandler'; describe('waitForCloseComplete', () => { beforeEach(() => { @@ -12,14 +12,14 @@ describe('waitForCloseComplete', () => { `; const onClose = vi.fn(); - waitForCloseComplete(onClose); + waitForCloseCompletion(onClose); expect(onClose).toHaveBeenCalledTimes(1); }); it('should trigger callback when closed state is added later', async () => { const onClose = vi.fn(); - waitForCloseComplete(onClose); + waitForCloseCompletion(onClose); const closedElement = document.createElement('span'); closedElement.className = 'State State--closed'; diff --git a/src/content/closeHandler.ts b/src/content/closeHandler.ts index a0863bf..3af9814 100644 --- a/src/content/closeHandler.ts +++ b/src/content/closeHandler.ts @@ -1,6 +1,6 @@ import { ShowSettings } from './showSettings'; import { isPullRequestPage } from './pageUtils'; -import { waitForCloseComplete } from './closeWatcher'; +import { CLOSE_DETECTION_TIMEOUT_MS, CLOSE_EXISTING_CHECK_DELAY_MS } from './constants'; export interface CloseHandlerOptions { showSettings: ShowSettings; @@ -26,9 +26,99 @@ export const detectCloseButtons = (options: CloseHandlerOptions): void => { ) { button.addEventListener('click', () => { console.log('🛑 Close pull request button clicked'); - waitForCloseComplete(() => options.onClosed()); + waitForCloseCompletion(options.onClosed); }); button.setAttribute('data-elden-ring-close-listener', 'true'); } }); }; + +export const waitForCloseCompletion = (onClosed: () => void): void => { + let closeHandled = false; + let checkTimeout: ReturnType | null = null; + let cleanupTimeout: ReturnType | null = null; + + const cleanup = (): void => { + if (checkTimeout) { + clearTimeout(checkTimeout); + checkTimeout = null; + } + if (cleanupTimeout) { + clearTimeout(cleanupTimeout); + cleanupTimeout = null; + } + }; + + const isClosedState = (element: Element | null): boolean => { + if (!element) return false; + if (!element.matches('.State.State--closed')) { + return false; + } + const text = element.textContent?.toLowerCase().trim() || ''; + return text.includes('closed'); + }; + + const handleClose = (observer: MutationObserver): void => { + if (closeHandled) return; + closeHandled = true; + console.log('☠️ Pull request closed!'); + cleanup(); + onClosed(); + observer.disconnect(); + }; + + const checkExistingClosedState = (observer: MutationObserver): boolean => { + const closedElement = document.querySelector('.State.State--closed'); + if (isClosedState(closedElement)) { + handleClose(observer); + return true; + } + return false; + }; + + const observer = new MutationObserver((mutations) => { + if (closeHandled) return; + + mutations.forEach((mutation) => { + mutation.addedNodes.forEach((node) => { + if (node.nodeType !== Node.ELEMENT_NODE || closeHandled) { + return; + } + + const element = node as Element; + if (isClosedState(element)) { + handleClose(observer); + return; + } + + const closedElement = element.querySelector('.State.State--closed'); + if (isClosedState(closedElement)) { + handleClose(observer); + } + }); + }); + }); + + observer.observe(document.body, { + childList: true, + subtree: true, + }); + + if (checkExistingClosedState(observer)) { + return; + } + + checkTimeout = setTimeout(() => { + if (!closeHandled) { + checkExistingClosedState(observer); + } + }, CLOSE_EXISTING_CHECK_DELAY_MS); + + cleanupTimeout = setTimeout(() => { + if (!closeHandled) { + cleanup(); + observer.disconnect(); + console.log('⏰ Close detection timeout'); + } + }, CLOSE_DETECTION_TIMEOUT_MS); +}; diff --git a/src/content/closeWatcher.ts b/src/content/closeWatcher.ts deleted file mode 100644 index a837304..0000000 --- a/src/content/closeWatcher.ts +++ /dev/null @@ -1,95 +0,0 @@ -let closeHandled = false; -// Track timeouts so we can cancel them once a close is confirmed -let checkTimeout: ReturnType | null = null; -let cleanupTimeout: ReturnType | null = null; - -const isClosedState = (element: Element | null): boolean => { - if (!element) return false; - if (!element.matches('.State.State--closed')) { - return false; - } - const text = element.textContent?.toLowerCase().trim() || ''; - return text.includes('closed'); -}; - -const handleClose = (observer: MutationObserver, onClose: () => void): void => { - if (closeHandled) return; - closeHandled = true; - console.log('☠️ Pull request closed!'); - if (checkTimeout) { - clearTimeout(checkTimeout); - checkTimeout = null; - } - if (cleanupTimeout) { - clearTimeout(cleanupTimeout); - cleanupTimeout = null; - } - onClose(); - observer.disconnect(); -}; - -const checkExistingClosedState = (observer: MutationObserver, onClose: () => void): boolean => { - const closedElement = document.querySelector('.State.State--closed'); - if (isClosedState(closedElement)) { - handleClose(observer, onClose); - return true; - } - return false; -}; - -export const waitForCloseComplete = (onClose: () => void, timeoutMs: number = 10000): void => { - closeHandled = false; - if (checkTimeout) { - clearTimeout(checkTimeout); - checkTimeout = null; - } - if (cleanupTimeout) { - clearTimeout(cleanupTimeout); - cleanupTimeout = null; - } - - const observer = new MutationObserver((mutations) => { - if (closeHandled) return; - - mutations.forEach((mutation) => { - mutation.addedNodes.forEach((node) => { - if (node.nodeType !== Node.ELEMENT_NODE || closeHandled) { - return; - } - - const element = node as Element; - if (isClosedState(element)) { - handleClose(observer, onClose); - return; - } - - const closedElement = element.querySelector('.State.State--closed'); - if (isClosedState(closedElement)) { - handleClose(observer, onClose); - } - }); - }); - }); - - observer.observe(document.body, { - childList: true, - subtree: true, - }); - - if (checkExistingClosedState(observer, onClose)) { - return; - } - - checkTimeout = setTimeout(() => { - if (!closeHandled) { - checkExistingClosedState(observer, onClose); - } - }, 100); - - cleanupTimeout = setTimeout(() => { - if (!closeHandled) { - observer.disconnect(); - console.log('⏰ Close detection timeout'); - } - }, timeoutMs); -}; diff --git a/src/content/constants.ts b/src/content/constants.ts new file mode 100644 index 0000000..1b13e87 --- /dev/null +++ b/src/content/constants.ts @@ -0,0 +1,2 @@ +export const CLOSE_EXISTING_CHECK_DELAY_MS = 100; +export const CLOSE_DETECTION_TIMEOUT_MS = 10000; diff --git a/src/content/content.ts b/src/content/content.ts index a866b00..356425c 100644 --- a/src/content/content.ts +++ b/src/content/content.ts @@ -6,8 +6,7 @@ import { CloseFeature, type GitHubFeature, } from './features'; -import { ShowSettings } from './showSettings'; -import { SettingsStore, type SettingsState } from './settingsStore'; +import { ShowSettings, type SettingsState } from './showSettings'; class EldenRingOrchestrator { private bannerShown: boolean = false; @@ -15,13 +14,11 @@ class EldenRingOrchestrator { private soundType: 'you-die-sound' | 'lost-grace-discovered' = 'you-die-sound'; private soundUrl: string; private features: GitHubFeature[] = []; - private settingsStore = new SettingsStore(); - private showSettings = new ShowSettings(() => this.settingsStore.getState()); + private showSettings = new ShowSettings(); constructor() { this.soundUrl = this.getSoundUrl(); - this.subscribeToSettings(); - this.settingsStore.init(); + this.showSettings.subscribe((state) => this.applySettings(state)); this.features = this.createFeatures(); this.init(); } @@ -34,10 +31,6 @@ class EldenRingOrchestrator { this.soundUrl = this.getSoundUrl(); } - private subscribeToSettings(): void { - this.settingsStore.subscribe((state) => this.applySettings(state)); - } - private applySettings(state: SettingsState): void { this.soundEnabled = state.soundEnabled; this.soundType = state.soundType; diff --git a/src/content/mergeHandler.test.ts b/src/content/mergeHandler.test.ts index 701dfc5..9ae8956 100644 --- a/src/content/mergeHandler.test.ts +++ b/src/content/mergeHandler.test.ts @@ -26,12 +26,17 @@ describe('mergeHandler', () => { }); const createShowSettings = (): ShowSettings => - new ShowSettings(() => ({ - showOnPRMerged: true, - showOnPRCreate: true, - showOnPRApprove: true, - showOnPRClose: true, - })); + new ShowSettings( + { + soundEnabled: true, + soundType: 'you-die-sound', + showOnPRMerged: true, + showOnPRCreate: true, + showOnPRApprove: true, + showOnPRClose: true, + }, + { autoInit: false }, + ); it('should trigger onMerged when merged state appears', () => { Object.defineProperty(window, 'location', { diff --git a/src/content/settingsStore.ts b/src/content/settingsStore.ts deleted file mode 100644 index d9142fa..0000000 --- a/src/content/settingsStore.ts +++ /dev/null @@ -1,96 +0,0 @@ -export interface SettingsState { - soundEnabled: boolean; - soundType: 'you-die-sound' | 'lost-grace-discovered'; - showOnPRMerged: boolean; - showOnPRCreate: boolean; - showOnPRApprove: boolean; - showOnPRClose: boolean; -} - -const defaultState: SettingsState = { - soundEnabled: true, - soundType: 'you-die-sound', - showOnPRMerged: true, - showOnPRCreate: true, - showOnPRApprove: true, - showOnPRClose: true, -}; - -export class SettingsStore { - private state: SettingsState = defaultState; - private subscribers: Array<(state: SettingsState) => void> = []; - - init(): void { - chrome.storage.sync.get( - [ - 'soundEnabled', - 'soundType', - 'showOnPRMerged', - 'showOnPRCreate', - 'showOnPRApprove', - 'showOnPRClose', - ], - (result) => { - this.state = { - soundEnabled: result.soundEnabled !== false, - soundType: result.soundType || 'you-die-sound', - showOnPRMerged: result.showOnPRMerged !== false, - showOnPRCreate: result.showOnPRCreate !== false, - showOnPRApprove: result.showOnPRApprove !== false, - showOnPRClose: result.showOnPRClose !== false, - }; - this.notify(); - }, - ); - - chrome.storage.onChanged.addListener((changes) => { - let updated = false; - const nextState = { ...this.state }; - - if (changes.soundEnabled) { - nextState.soundEnabled = changes.soundEnabled.newValue; - updated = true; - } - if (changes.soundType) { - nextState.soundType = changes.soundType.newValue; - updated = true; - } - if (changes.showOnPRMerged) { - nextState.showOnPRMerged = changes.showOnPRMerged.newValue; - updated = true; - } - if (changes.showOnPRCreate) { - nextState.showOnPRCreate = changes.showOnPRCreate.newValue; - updated = true; - } - if (changes.showOnPRApprove) { - nextState.showOnPRApprove = changes.showOnPRApprove.newValue; - updated = true; - } - if (changes.showOnPRClose) { - nextState.showOnPRClose = changes.showOnPRClose.newValue; - updated = true; - } - - if (updated) { - this.state = nextState; - this.notify(); - } - }); - } - - getState(): SettingsState { - return this.state; - } - - subscribe(listener: (state: SettingsState) => void): () => void { - this.subscribers.push(listener); - return () => { - this.subscribers = this.subscribers.filter((cb) => cb !== listener); - }; - } - - private notify(): void { - this.subscribers.forEach((listener) => listener(this.state)); - } -} diff --git a/src/content/showSettings.ts b/src/content/showSettings.ts index 84c18fe..1caadc7 100644 --- a/src/content/showSettings.ts +++ b/src/content/showSettings.ts @@ -1,4 +1,11 @@ -import type { SettingsState } from './settingsStore'; +export interface SettingsState { + soundEnabled: boolean; + soundType: 'you-die-sound' | 'lost-grace-discovered'; + showOnPRMerged: boolean; + showOnPRCreate: boolean; + showOnPRApprove: boolean; + showOnPRClose: boolean; +} export type ShowSettingKey = 'merged' | 'created' | 'approved' | 'closed'; @@ -14,11 +21,107 @@ const STATE_KEY_MAP: Record = { closed: 'showOnPRClose', }; +const defaultState: SettingsState = { + soundEnabled: true, + soundType: 'you-die-sound', + showOnPRMerged: true, + showOnPRCreate: true, + showOnPRApprove: true, + showOnPRClose: true, +}; + export class ShowSettings { - constructor(private stateProvider: () => ShowSettingsFlags) {} + private state: SettingsState; + private subscribers: Array<(state: SettingsState) => void> = []; + private autoInit: boolean; + + constructor(initialState?: Partial, options?: { autoInit?: boolean }) { + this.state = { ...defaultState, ...initialState }; + this.autoInit = options?.autoInit ?? true; + + if (this.autoInit) { + this.initStorageSync(); + } + } + + private initStorageSync(): void { + chrome.storage.sync.get( + [ + 'soundEnabled', + 'soundType', + 'showOnPRMerged', + 'showOnPRCreate', + 'showOnPRApprove', + 'showOnPRClose', + ], + (result) => { + this.state = { + soundEnabled: result.soundEnabled !== false, + soundType: result.soundType || 'you-die-sound', + showOnPRMerged: result.showOnPRMerged !== false, + showOnPRCreate: result.showOnPRCreate !== false, + showOnPRApprove: result.showOnPRApprove !== false, + showOnPRClose: result.showOnPRClose !== false, + }; + this.notify(); + }, + ); + + chrome.storage.onChanged.addListener((changes) => { + let updated = false; + const nextState = { ...this.state }; + + if (changes.soundEnabled) { + nextState.soundEnabled = changes.soundEnabled.newValue; + updated = true; + } + if (changes.soundType) { + nextState.soundType = changes.soundType.newValue; + updated = true; + } + if (changes.showOnPRMerged) { + nextState.showOnPRMerged = changes.showOnPRMerged.newValue; + updated = true; + } + if (changes.showOnPRCreate) { + nextState.showOnPRCreate = changes.showOnPRCreate.newValue; + updated = true; + } + if (changes.showOnPRApprove) { + nextState.showOnPRApprove = changes.showOnPRApprove.newValue; + updated = true; + } + if (changes.showOnPRClose) { + nextState.showOnPRClose = changes.showOnPRClose.newValue; + updated = true; + } + + if (updated) { + this.state = nextState; + this.notify(); + } + }); + } + + getState(): SettingsState { + return this.state; + } + + subscribe(listener: (state: SettingsState) => void): () => void { + this.subscribers.push(listener); + listener(this.state); + return () => { + this.subscribers = this.subscribers.filter((cb) => cb !== listener); + }; + } + + private notify(): void { + this.subscribers.forEach((listener) => listener(this.state)); + } isEnabled(setting: ShowSettingKey): boolean { const key = STATE_KEY_MAP[setting]; - return this.stateProvider()[key]; + const flags = this.state as ShowSettingsFlags; + return flags[key]; } }