From 647a32fbac04dd5ab3452bed3c63037c51d07cc2 Mon Sep 17 00:00:00 2001 From: SethBurkart123 Date: Sun, 30 Mar 2025 12:04:39 +1100 Subject: [PATCH] fix: settings props not being correctly set --- src/interface/components/Slider.svelte | 21 ++- src/interface/pages/settings/general.svelte | 105 ++++++------ .../built-in/animated-background/index.ts | 26 +-- src/plugins/built-in/test/index.ts | 22 ++- src/plugins/core/createAPI.ts | 154 ++++++------------ src/plugins/core/manager.ts | 34 ++-- src/plugins/core/settings.ts | 111 +++---------- src/plugins/core/settingsHelpers.ts | 50 ++++++ src/plugins/core/types.ts | 4 +- 9 files changed, 247 insertions(+), 280 deletions(-) create mode 100644 src/plugins/core/settingsHelpers.ts diff --git a/src/interface/components/Slider.svelte b/src/interface/components/Slider.svelte index 00459c70..2dd465f6 100644 --- a/src/interface/components/Slider.svelte +++ b/src/interface/components/Slider.svelte @@ -1,13 +1,24 @@ -
+
onChange(Number(e.currentTarget.value))} diff --git a/src/interface/pages/settings/general.svelte b/src/interface/pages/settings/general.svelte index 61db53ab..0375969f 100644 --- a/src/interface/pages/settings/general.svelte +++ b/src/interface/pages/settings/general.svelte @@ -12,21 +12,24 @@ import hideSensitiveContent from "@/seqta/ui/dev/hideSensitiveContent" import { getAllPluginSettings } from "@/plugins" + import type { BooleanSetting, StringSetting, NumberSetting, SelectSetting } from "@/plugins/core/types" - interface PluginSetting { - id: string; - title: string; - description?: string; - type: string; - default: any; - options?: Array<{value: string, label: string}>; - } + // Union type representing all possible settings + type SettingType = + (Omit & { type: 'boolean', id: string }) | + (Omit & { type: 'string', id: string }) | + (Omit & { type: 'number', id: string }) | + (Omit, 'type'> & { + type: 'select', + id: string, + options: Array<{ value: string, label: string }> + }); interface Plugin { pluginId: string; name: string; description: string; - settings: Record; + settings: Record; } const pluginSettings = getAllPluginSettings() as Plugin[]; @@ -75,10 +78,10 @@ function getPluginSettingEntries() { const entries: any[] = []; - + pluginSettings.forEach(plugin => { if (Object.keys(plugin.settings).length === 0) return; - + // Add enable/disable toggle if plugin has disableToggle set if ((plugin as any).disableToggle) { entries.push({ @@ -90,37 +93,61 @@ state: pluginSettingsValues[plugin.pluginId]?.enabled ?? true, onChange: (value: boolean) => { updatePluginSetting(plugin.pluginId, 'enabled', value); - // The plugin manager will handle the actual enabling/disabling } } }); } - + Object.entries(plugin.settings).forEach(([key, setting]) => { const id = getPluginSettingId(plugin.pluginId, key); - + + const props: Record = { + state: pluginSettingsValues[plugin.pluginId]?.[key] ?? setting.default, + onChange: (value: any) => { + if (setting.type === 'number' && typeof value === 'string') { + value = parseFloat(value); + } + updatePluginSetting(plugin.pluginId, key, value); + } + }; + + if (setting.type === 'number') { + if (typeof setting.min === 'number') props.min = setting.min; + if (typeof setting.max === 'number') props.max = setting.max; + if (typeof setting.step === 'number') props.step = setting.step; + } + + if (setting.type === 'select') { + props.options = setting.options; + } + + let Component; + switch (setting.type) { + case 'boolean': + Component = Switch; + break; + case 'number': + Component = Slider; + break; + case 'select': + Component = Select; + break; + default: + Component = null; + } + + if (!Component) return; + entries.push({ title: setting.title || key, description: setting.description || '', id, - Component: setting.type === 'boolean' ? Switch : - setting.type === 'select' ? Select : - setting.type === 'number' ? Slider : - setting.type === 'string' ? (setting.options ? Select : null) : Switch, - props: { - state: pluginSettingsValues[plugin.pluginId]?.[key] ?? setting.default, - onChange: (value: any) => { - if (setting.type === 'number' && typeof value === 'string') { - value = parseFloat(value); - } - updatePluginSetting(plugin.pluginId, key, value); - }, - options: setting.options - } + Component, + props }); }); }); - + return entries; } @@ -155,26 +182,6 @@ onChange: (isOn: boolean) => settingsState.transparencyEffects = isOn } }, - { - title: "Animated Background", - description: "Adds an animated background to BetterSEQTA. (May impact battery life)", - id: 2, - Component: Switch, - props: { - state: $settingsState.animatedbk, - onChange: (isOn: boolean) => settingsState.animatedbk = isOn - } - }, - { - title: "Animated Background Speed", - description: "Controls the speed of the animated background.", - id: 3, - Component: Slider, - props: { - state: $settingsState.bksliderinput, - onChange: (value: number) => settingsState.bksliderinput = `${value}` - } - }, { title: "Custom Theme Colour", description: "Customise the overall theme colour of SEQTA Learn.", diff --git a/src/plugins/built-in/animated-background/index.ts b/src/plugins/built-in/animated-background/index.ts index b3eb00c6..e3907b84 100644 --- a/src/plugins/built-in/animated-background/index.ts +++ b/src/plugins/built-in/animated-background/index.ts @@ -1,28 +1,34 @@ -import type { Plugin } from '../../core/types'; +import { BasePlugin } from '../../core/settings'; +import { type Plugin } from '@/plugins/core/types'; +import { defineSettings, numberSetting, Setting } from '@/plugins/core/settingsHelpers'; import styles from './styles.css?inline'; -import { BasePlugin, NumberSetting } from '../../core/settings'; -class AnimatedBackgroundPluginClass extends BasePlugin { - @NumberSetting({ +const settings = defineSettings({ + speed: numberSetting({ default: 1, title: "Animation Speed", - description: "Controls the speed of the animated background", + description: "Controls how fast the background moves", min: 0.1, - max: 2 + max: 2, + step: 0.05 }) +}); + +class AnimatedBackgroundPluginClass extends BasePlugin { + @Setting(settings.speed) speed!: number; } -const settingsInstance = new AnimatedBackgroundPluginClass(); +const instance = new AnimatedBackgroundPluginClass(); -const animatedBackgroundPlugin: Plugin = { +const animatedBackgroundPlugin: Plugin = { id: 'animated-background', name: 'Animated Background', description: 'Adds an animated background to BetterSEQTA+', version: '1.0.0', disableToggle: true, - styles, - settings: settingsInstance.settings, + styles: styles, + settings: instance.settings, run: async (api) => { // Create the background elements diff --git a/src/plugins/built-in/test/index.ts b/src/plugins/built-in/test/index.ts index d77bc5c1..0bab39cf 100644 --- a/src/plugins/built-in/test/index.ts +++ b/src/plugins/built-in/test/index.ts @@ -1,18 +1,26 @@ -import type { Plugin } from '../../core/types'; -import { BasePlugin, BooleanSetting } from '../../core/settings'; +import type { Plugin } from '@/plugins/core/types'; +import { BasePlugin } from '@/plugins/core/settings'; +import { defineSettings, booleanSetting, Setting } from '@/plugins/core/settingsHelpers'; -class TestPluginClass extends BasePlugin { - @BooleanSetting({ +// Step 1: Define settings with proper typing +const settings = defineSettings({ + someSetting: booleanSetting({ default: true, title: "Test Plugin", description: "Some random setting", }) +}); + +// Step 2: Create the plugin class with @Setting decorators +class TestPluginClass extends BasePlugin { + @Setting(settings.someSetting) someSetting!: boolean; } +// Step 3: Instantiate and plug it in const settingsInstance = new TestPluginClass(); -const testPlugin: Plugin = { +const testPlugin: Plugin = { id: 'test', name: 'Test Plugin', description: 'A test plugin for BetterSEQTA+', @@ -24,6 +32,8 @@ const testPlugin: Plugin = { const { unregister } = api.seqta.onPageChange((page) => { console.log('Page changed to', page); + + console.log('Current setting value:', api.settings.someSetting); }); return () => { @@ -33,4 +43,4 @@ const testPlugin: Plugin = { } }; -export default testPlugin; \ No newline at end of file +export default testPlugin; diff --git a/src/plugins/core/createAPI.ts b/src/plugins/core/createAPI.ts index 6233c2eb..ca5fd467 100644 --- a/src/plugins/core/createAPI.ts +++ b/src/plugins/core/createAPI.ts @@ -1,4 +1,4 @@ -import type { EventsAPI, Plugin, PluginAPI, PluginSettings, SEQTAAPI, SettingsAPI, StorageAPI } from './types'; +import type { EventsAPI, Plugin, PluginAPI, PluginSettings, SEQTAAPI, SettingsAPI, SettingValue, StorageAPI } from './types'; import { eventManager } from '@/seqta/utils/listeners/EventManager'; import ReactFiber from '@/seqta/utils/ReactFiber'; import browser from 'webextension-polyfill'; @@ -41,28 +41,17 @@ function createSEQTAAPI(): SEQTAAPI { function createSettingsAPI(plugin: Plugin): SettingsAPI & { loaded: Promise } { const storageKey = `plugin.${plugin.id}.settings`; - - // Use SettingValue to properly type the listeners - // This ensures callbacks get correctly typed parameters - const listeners = new Map) => void>>(); - + const listeners = new Map void>>(); let settings: { [K in keyof T]: SettingValue }; const storageListeners = new Set<(changes: { [key: string]: any }, area: string) => void>(); - // Initialize settings with defaults and proper typing - settings = Object.entries(plugin.settings).reduce((acc, [key, setting]) => { - // Extract the value from the default based on the setting type - if (setting.type === 'boolean') { - acc[key as keyof T] = setting.default as SettingValue; - } else if (setting.type === 'number') { - acc[key as keyof T] = setting.default as SettingValue; - } else if (setting.type === 'string') { - acc[key as keyof T] = setting.default as SettingValue; - } else if (setting.type === 'select') { - acc[key as keyof T] = setting.default as SettingValue; - } - return acc; - }, {} as { [K in keyof T]: SettingValue }); + // Initialize settings with defaults + const defaultSettings = {} as { [K in keyof T]: SettingValue }; + for (const key in plugin.settings) { + defaultSettings[key] = plugin.settings[key].default as SettingValue; + } + settings = defaultSettings; + // Create a promise that resolves when settings are loaded const loaded = (async () => { @@ -71,22 +60,9 @@ function createSettingsAPI(plugin: Plugin): Setting if (stored[storageKey]) { Object.entries(stored[storageKey]).forEach(([key, value]) => { if (key in settings) { - // Use proper type assertion based on the setting type - const settingType = plugin.settings[key as keyof T].type; - if (settingType === 'boolean' && typeof value === 'boolean') { - settings[key as keyof T] = value as SettingValue; - } else if (settingType === 'number' && typeof value === 'number') { - settings[key as keyof T] = value as SettingValue; - } else if (settingType === 'string' && typeof value === 'string') { - settings[key as keyof T] = value as SettingValue; - } else if (settingType === 'select' && typeof value === 'string') { - settings[key as keyof T] = value as SettingValue; - } - + settings[key as keyof T] = value as any; // Notify any listeners that might have been registered already - listeners.get(key as keyof T)?.forEach(callback => - callback(settings[key as keyof T] as SettingValue) - ); + listeners.get(key as keyof T)?.forEach(callback => callback(value)); } }); } @@ -102,24 +78,8 @@ function createSettingsAPI(plugin: Plugin): Setting if (newValue) { // Update settings and notify listeners Object.entries(newValue).forEach(([key, value]) => { - if (key in settings) { - // Use proper type assertion based on the setting type - const settingType = plugin.settings[key as keyof T].type; - if (settingType === 'boolean' && typeof value === 'boolean') { - settings[key as keyof T] = value as SettingValue; - } else if (settingType === 'number' && typeof value === 'number') { - settings[key as keyof T] = value as SettingValue; - } else if (settingType === 'string' && typeof value === 'string') { - settings[key as keyof T] = value as SettingValue; - } else if (settingType === 'select' && typeof value === 'string') { - settings[key as keyof T] = value as SettingValue; - } - - // Notify listeners with the correctly typed value - listeners.get(key as keyof T)?.forEach(callback => - callback(settings[key as keyof T] as SettingValue) - ); - } + settings[key as keyof T] = value as any; + listeners.get(key as keyof T)?.forEach(callback => callback(value)); }); } } @@ -127,57 +87,45 @@ function createSettingsAPI(plugin: Plugin): Setting browser.storage.onChanged.addListener(handleStorageChange); storageListeners.add(handleStorageChange); - // Create a proxy to handle direct property access - const proxy = new Proxy(settings, { - get(target, prop: string) { - if (prop === 'onChange') { - return (key: K, callback: (value: SettingValue) => void) => { - if (!listeners.has(key)) { - listeners.set(key, new Set()); - } - listeners.get(key)!.add(callback as (value: SettingValue) => void); - return { - unregister: () => { - listeners.get(key)?.delete(callback as (value: SettingValue) => void); - } - }; - }; - } - if (prop === 'loaded') { - return loaded; - } - return target[prop as keyof T]; - }, - set(target, prop: string, value: any) { - if (prop === 'onChange' || prop === 'offChange' || prop === 'loaded') return false; - - // Try to apply the right type based on the setting definition - if (prop in plugin.settings) { - const settingType = plugin.settings[prop as keyof T].type; - if (settingType === 'boolean' && typeof value === 'boolean') { - target[prop as keyof T] = value as SettingValue; - } else if (settingType === 'number' && typeof value === 'number') { - target[prop as keyof T] = value as SettingValue; - } else if (settingType === 'string' && typeof value === 'string') { - target[prop as keyof T] = value as SettingValue; - } else if (settingType === 'select' && typeof value === 'string') { - target[prop as keyof T] = value as SettingValue; - } - } - - // Store all settings under the plugin's settings key - browser.storage.local.set({ - [storageKey]: target - }); - - // Notify listeners - listeners.get(prop as keyof T)?.forEach(callback => - callback(target[prop as keyof T] as SettingValue) - ); - return true; - }, - }) as SettingsAPI & { loaded: Promise }; + const baseSettings = {} as { [K in keyof T]: SettingValue }; + for (const key in plugin.settings) { + baseSettings[key] = plugin.settings[key].default as SettingValue; + } + const settingsWithMeta = { + ...baseSettings, + onChange: (key: K, callback: (value: SettingValue) => void) => { + if (!listeners.has(key)) { + listeners.set(key, new Set()); + } + listeners.get(key)!.add(callback); + return { + unregister: () => { + listeners.get(key)!.delete(callback); + } + }; + }, + offChange: (key: K, callback: (value: SettingValue) => void) => { + listeners.get(key)?.delete(callback); + }, + loaded + }; + + const proxy = new Proxy(settingsWithMeta, { + get(target, prop) { + return target[prop as keyof typeof target]; + }, + set(target, prop, value) { + if (prop === 'onChange' || prop === 'offChange' || prop === 'loaded') return false; + + target[prop as keyof T] = value; + browser.storage.local.set({ [storageKey]: baseSettings }); // Only store base settings + listeners.get(prop as keyof T)?.forEach(callback => callback(value)); + return true; + } + }) as SettingsAPI; + + return proxy; } diff --git a/src/plugins/core/manager.ts b/src/plugins/core/manager.ts index 600f49c4..b9879fd5 100644 --- a/src/plugins/core/manager.ts +++ b/src/plugins/core/manager.ts @@ -1,4 +1,4 @@ -import type { Plugin, PluginSettings } from './types'; +import type { Plugin, PluginSettings, BooleanSetting, StringSetting, NumberSetting, SelectSetting } from './types'; import { createPluginAPI } from './createAPI'; import browser from 'webextension-polyfill'; @@ -164,25 +164,29 @@ export class PluginManager { public getAllPluginSettings(): Array<{ pluginId: string; name: string; + description: string; settings: { - [key: string]: { - id: string; - title: string; - description?: string; - type: string; - default: any; - } + [key: string]: (Omit & { type: 'boolean', id: string }) | + (Omit & { type: 'string', id: string }) | + (Omit & { type: 'number', id: string }) | + (Omit, 'type'> & { type: 'select', id: string, options: Array<{ value: string, label: string }> }); } }> { return Array.from(this.plugins.entries()).map(([id, plugin]) => { const settingsEntries = Object.entries(plugin.settings).map(([key, setting]) => { - return [key, { - id: key, - title: (setting as any).title || key, - description: (setting as any).description || '', - type: (setting as any).type, - default: (setting as any).default - }]; + const settingObj = setting as any; + // Create a copy of the setting object without any functions + const result: any = Object.fromEntries( + Object.entries(settingObj) + .filter(([_, value]) => typeof value !== 'function') + ); + + // Ensure required properties are present + result.id = key; + result.title = result.title || key; + result.description = result.description || ''; + + return [key, result]; }); if (plugin.disableToggle) { diff --git a/src/plugins/core/settings.ts b/src/plugins/core/settings.ts index 38faf4d6..129bea79 100644 --- a/src/plugins/core/settings.ts +++ b/src/plugins/core/settings.ts @@ -1,108 +1,39 @@ import type { PluginSettings } from './types'; -// Base interfaces for our settings -interface BaseSettingOptions { - title: string; - description?: string; -} - -interface BooleanSettingOptions extends BaseSettingOptions { - default: boolean; -} - -interface StringSettingOptions extends BaseSettingOptions { - default: string; - maxLength?: number; - pattern?: string; -} - -interface NumberSettingOptions extends BaseSettingOptions { - default: number; - min?: number; - max?: number; - step?: number; -} - -interface SelectSettingOptions extends BaseSettingOptions { - default: T; - options: readonly T[]; -} - -// The actual decorators -export function BooleanSetting(options: BooleanSettingOptions): PropertyDecorator { - return (target: Object, propertyKey: string | symbol) => { - // Ensure the settings property exists on the constructor's prototype +export function Setting(settingDef: any): PropertyDecorator { + return (target, propertyKey) => { const proto = target.constructor.prototype; if (!proto.hasOwnProperty('settings')) { - proto.settings = {}; + Object.defineProperty(proto, 'settings', { + value: {}, + writable: true, + configurable: true, + enumerable: true + }); } - - // Add the setting to the prototype's settings object with const assertion - proto.settings[propertyKey] = { - type: 'boolean' as const, - ...options - }; - }; -} -export function StringSetting(options: StringSettingOptions): PropertyDecorator { - return (target: Object, propertyKey: string | symbol) => { - // Ensure the settings property exists on the constructor's prototype - const proto = target.constructor.prototype; - if (!proto.hasOwnProperty('settings')) { - proto.settings = {}; - } - - // Add the setting to the prototype's settings object with const assertion - proto.settings[propertyKey] = { - type: 'string' as const, - ...options - }; - }; -} - -export function NumberSetting(options: NumberSettingOptions): PropertyDecorator { - return (target: Object, propertyKey: string | symbol) => { - // Ensure the settings property exists on the constructor's prototype - const proto = target.constructor.prototype; - if (!proto.hasOwnProperty('settings')) { - proto.settings = {}; - } - - // Add the setting to the prototype's settings object with const assertion - proto.settings[propertyKey] = { - type: 'number' as const, - ...options - }; - }; -} - -export function SelectSetting(options: SelectSettingOptions): PropertyDecorator { - return (target: Object, propertyKey: string | symbol) => { - // Ensure the settings property exists on the constructor's prototype - const proto = target.constructor.prototype; - if (!proto.hasOwnProperty('settings')) { - proto.settings = {}; - } - - // Add the setting to the prototype's settings object with const assertion - proto.settings[propertyKey] = { - type: 'select' as const, - ...options - }; + proto.settings[propertyKey] = settingDef; }; } // Base plugin class that handles settings export abstract class BasePlugin { // The settings property will be populated by decorators - settings!: T; - + // Keep the instance property and constructor logic as is, + // as changing it would require changing animated-background/index.ts + settings!: T; // Use definite assignment assertion + constructor() { // Copy settings from the prototype to the instance // This ensures that each instance has its own settings object - if (this.constructor.prototype.settings) { + // IMPORTANT: Ensure the prototype actually HAS settings before copying + if (this.constructor.prototype.hasOwnProperty('settings')) { + // Deep clone might be safer if settings objects become complex, + // but a shallow clone is usually fine for this structure. this.settings = { ...this.constructor.prototype.settings } as T; + } else { + // Fallback if decorators somehow didn't run or add the property + this.settings = {} as T; } } -} \ No newline at end of file +} \ No newline at end of file diff --git a/src/plugins/core/settingsHelpers.ts b/src/plugins/core/settingsHelpers.ts new file mode 100644 index 00000000..5db3188f --- /dev/null +++ b/src/plugins/core/settingsHelpers.ts @@ -0,0 +1,50 @@ +import type { NumberSetting, BooleanSetting, StringSetting, SelectSetting } from './types'; + +export function numberSetting(options: Omit): NumberSetting { + return { + type: 'number', + ...options + }; +} + +export function booleanSetting(options: Omit): BooleanSetting { + return { + type: 'boolean', + ...options + }; +} + +export function stringSetting(options: Omit): StringSetting { + return { + type: 'string', + ...options + }; +} + +export function selectSetting(options: Omit, 'type'>): SelectSetting { + return { + type: 'select', + ...options + }; +} + +export function defineSettings>(settings: T): T { + return settings; +} + +export function Setting(settingDef: any): PropertyDecorator { + return (target, propertyKey) => { + const proto = target.constructor.prototype; + if (!proto.hasOwnProperty('settings')) { + Object.defineProperty(proto, 'settings', { + value: {}, + writable: true, + configurable: true, + enumerable: true + }); + } + + proto.settings[propertyKey] = settingDef; + }; +} + \ No newline at end of file diff --git a/src/plugins/core/types.ts b/src/plugins/core/types.ts index f2a2a382..c5f48f01 100644 --- a/src/plugins/core/types.ts +++ b/src/plugins/core/types.ts @@ -52,8 +52,8 @@ export type SettingsAPI = { } & { onChange: (key: K, callback: (value: SettingValue) => void) => { unregister: () => void }; offChange: (key: K, callback: (value: SettingValue) => void) => void; - loaded: Promise; // Promise that resolves when settings are loaded -} + loaded: Promise; +} export interface SEQTAAPI { onMount: (selector: string, callback: (element: Element) => void) => { unregister: () => void };