From 2a8276c5469cbdbd1677eb45ee39861c271aec32 Mon Sep 17 00:00:00 2001 From: David Tejada Date: Tue, 17 Sep 2024 10:17:56 +0200 Subject: [PATCH 1/5] Refactor settings watcher in content scripts to reduce decoupling --- package-lock.json | 55 +++++++-- package.json | 1 + src/content/actions/keyboardClicking.ts | 20 ++++ .../actions/runActionOnTextMatchedElement.ts | 4 +- src/content/actions/scroll.ts | 4 +- src/content/hints/HintClass.ts | 35 +++--- src/content/hints/selectors.ts | 7 ++ src/content/notify/Toast.tsx | 6 +- src/content/notify/notify.tsx | 12 +- src/content/observe.ts | 24 +++- src/content/settings/cacheSettings.ts | 17 --- src/content/settings/settingsManager.ts | 83 +++++++++++++ src/content/settings/toggles.ts | 4 +- src/content/settings/watchSettingsChanges.ts | 113 ------------------ src/content/setup/initContentScript.ts | 8 +- src/content/utils/decorateTitle.ts | 26 +++- src/content/wrappers/ElementWrapperClass.ts | 6 +- src/content/wrappers/refresh.ts | 32 +++++ src/typings/TypingUtils.ts | 7 +- 19 files changed, 273 insertions(+), 191 deletions(-) delete mode 100644 src/content/settings/cacheSettings.ts create mode 100644 src/content/settings/settingsManager.ts delete mode 100644 src/content/settings/watchSettingsChanges.ts diff --git a/package-lock.json b/package-lock.json index f2424117..c1f1862a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,6 +13,7 @@ "color": "^4.2.3", "combinations": "^1.0.0", "css-selector-generator": "^3.6.4", + "emittery": "^1.0.3", "fuse.js": "^6.6.2", "intersect": "^1.0.1", "lodash": "^4.17.21", @@ -6730,12 +6731,11 @@ "dev": true }, "node_modules/emittery": { - "version": "0.13.1", - "resolved": "https://registry.npmjs.org/emittery/-/emittery-0.13.1.tgz", - "integrity": "sha512-DeWwawk6r5yR9jFgnDKYt4sLS0LmHJJi3ZOnb5/JdbYwj3nW+FxQnHIjhBKz8YLC7oRNPVM9NQ47I3CVx34eqQ==", - "dev": true, + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/emittery/-/emittery-1.0.3.tgz", + "integrity": "sha512-tJdCJitoy2lrC2ldJcqN4vkqJ00lT+tOWNT1hBJjO/3FDMJa5TTIiYGCKGkn/WfCyOzUMObeohbVTj00fhiLiA==", "engines": { - "node": ">=12" + "node": ">=14.16" }, "funding": { "url": "https://github.com/sindresorhus/emittery?sponsor=1" @@ -10933,6 +10933,18 @@ "integrity": "sha512-Kvp459HrV2FEJ1CAsi1Ku+MY3kasH19TFykTz2xWmMeq6bk2NU3XXvfJ+Q61m0xktWwt+1HSYf3JZsTms3aRJg==", "dev": true }, + "node_modules/jest-runner/node_modules/emittery": { + "version": "0.13.1", + "resolved": "https://registry.npmjs.org/emittery/-/emittery-0.13.1.tgz", + "integrity": "sha512-DeWwawk6r5yR9jFgnDKYt4sLS0LmHJJi3ZOnb5/JdbYwj3nW+FxQnHIjhBKz8YLC7oRNPVM9NQ47I3CVx34eqQ==", + "dev": true, + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/sindresorhus/emittery?sponsor=1" + } + }, "node_modules/jest-runner/node_modules/jest-haste-map": { "version": "29.4.3", "resolved": "https://registry.npmjs.org/jest-haste-map/-/jest-haste-map-29.4.3.tgz", @@ -11452,6 +11464,18 @@ "node": "^14.15.0 || ^16.10.0 || >=18.0.0" } }, + "node_modules/jest-watcher/node_modules/emittery": { + "version": "0.13.1", + "resolved": "https://registry.npmjs.org/emittery/-/emittery-0.13.1.tgz", + "integrity": "sha512-DeWwawk6r5yR9jFgnDKYt4sLS0LmHJJi3ZOnb5/JdbYwj3nW+FxQnHIjhBKz8YLC7oRNPVM9NQ47I3CVx34eqQ==", + "dev": true, + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/sindresorhus/emittery?sponsor=1" + } + }, "node_modules/jest-worker": { "version": "27.5.1", "resolved": "https://registry.npmjs.org/jest-worker/-/jest-worker-27.5.1.tgz", @@ -24182,10 +24206,9 @@ "dev": true }, "emittery": { - "version": "0.13.1", - "resolved": "https://registry.npmjs.org/emittery/-/emittery-0.13.1.tgz", - "integrity": "sha512-DeWwawk6r5yR9jFgnDKYt4sLS0LmHJJi3ZOnb5/JdbYwj3nW+FxQnHIjhBKz8YLC7oRNPVM9NQ47I3CVx34eqQ==", - "dev": true + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/emittery/-/emittery-1.0.3.tgz", + "integrity": "sha512-tJdCJitoy2lrC2ldJcqN4vkqJ00lT+tOWNT1hBJjO/3FDMJa5TTIiYGCKGkn/WfCyOzUMObeohbVTj00fhiLiA==" }, "emoji-regex": { "version": "8.0.0", @@ -27243,6 +27266,12 @@ "integrity": "sha512-Kvp459HrV2FEJ1CAsi1Ku+MY3kasH19TFykTz2xWmMeq6bk2NU3XXvfJ+Q61m0xktWwt+1HSYf3JZsTms3aRJg==", "dev": true }, + "emittery": { + "version": "0.13.1", + "resolved": "https://registry.npmjs.org/emittery/-/emittery-0.13.1.tgz", + "integrity": "sha512-DeWwawk6r5yR9jFgnDKYt4sLS0LmHJJi3ZOnb5/JdbYwj3nW+FxQnHIjhBKz8YLC7oRNPVM9NQ47I3CVx34eqQ==", + "dev": true + }, "jest-haste-map": { "version": "29.4.3", "resolved": "https://registry.npmjs.org/jest-haste-map/-/jest-haste-map-29.4.3.tgz", @@ -27660,6 +27689,14 @@ "emittery": "^0.13.1", "jest-util": "^29.4.3", "string-length": "^4.0.1" + }, + "dependencies": { + "emittery": { + "version": "0.13.1", + "resolved": "https://registry.npmjs.org/emittery/-/emittery-0.13.1.tgz", + "integrity": "sha512-DeWwawk6r5yR9jFgnDKYt4sLS0LmHJJi3ZOnb5/JdbYwj3nW+FxQnHIjhBKz8YLC7oRNPVM9NQ47I3CVx34eqQ==", + "dev": true + } } }, "jest-worker": { diff --git a/package.json b/package.json index 185fd480..3d3a201b 100644 --- a/package.json +++ b/package.json @@ -154,6 +154,7 @@ "color": "^4.2.3", "combinations": "^1.0.0", "css-selector-generator": "^3.6.4", + "emittery": "^1.0.3", "fuse.js": "^6.6.2", "intersect": "^1.0.1", "lodash": "^4.17.21", diff --git a/src/content/actions/keyboardClicking.ts b/src/content/actions/keyboardClicking.ts index 7a987f9d..30934861 100644 --- a/src/content/actions/keyboardClicking.ts +++ b/src/content/actions/keyboardClicking.ts @@ -2,6 +2,9 @@ import browser from "webextension-polyfill"; import { getHintsInTab } from "../utils/getHintsInTab"; import { getHintedWrappers } from "../wrappers/wrappers"; import { isEditable, getActiveElement } from "../utils/domUtils"; +import { onSettingChange } from "../settings/settingsManager"; +import { notify } from "../notify/notify"; +import { refresh } from "../wrappers/refresh"; let keysPressedBuffer = ""; let timeoutId: ReturnType; @@ -102,3 +105,20 @@ export function initKeyboardClicking() { export function stopKeyboardClicking() { window.removeEventListener("keydown", keydownHandler, true); } + +onSettingChange("keyboardClicking", async (keyboardClicking) => { + if (keyboardClicking) { + initKeyboardClicking(); + } else { + stopKeyboardClicking(); + } + + const status = keyboardClicking ? "enabled" : "disabled"; + + await notify(`Keyboard clicking ${status}`, { + icon: status, + toastId: "keyboardToggle", + }); + + await refresh({ hintsCharacters: true }); +}); diff --git a/src/content/actions/runActionOnTextMatchedElement.ts b/src/content/actions/runActionOnTextMatchedElement.ts index 2d80f3cf..5f1e0e88 100644 --- a/src/content/actions/runActionOnTextMatchedElement.ts +++ b/src/content/actions/runActionOnTextMatchedElement.ts @@ -1,6 +1,6 @@ import Fuse from "fuse.js"; import { RangoActionWithTargets } from "../../typings/RangoAction"; -import { getCachedSetting } from "../settings/cacheSettings"; +import { getSetting } from "../settings/settingsManager"; import { getToggles } from "../settings/toggles"; import { deepGetElements } from "../utils/deepGetElements"; import { isHintable } from "../utils/isHintable"; @@ -24,7 +24,7 @@ function trimTextContent(element: Element) { async function getHintables() { // Hints are on, there will be wrappers for the hintable elements - if (getToggles().computed || getCachedSetting("alwaysComputeHintables")) { + if (getToggles().computed || getSetting("alwaysComputeHintables")) { return getAllWrappers() .filter((wrapper) => wrapper.isHintable && isVisible(wrapper.element)) .map((wrapper) => ({ diff --git a/src/content/actions/scroll.ts b/src/content/actions/scroll.ts index 7fa0a9f0..1c057dd6 100644 --- a/src/content/actions/scroll.ts +++ b/src/content/actions/scroll.ts @@ -1,7 +1,7 @@ import { isHtmlElement } from "../../typings/TypingUtils"; -import { getCachedSetting } from "../settings/cacheSettings"; import { getUserScrollableContainer } from "../utils/getUserScrollableContainer"; import { ElementWrapper } from "../../typings/ElementWrapper"; +import { getSetting } from "../settings/settingsManager"; const DEFAULT_SCROLL_FACTOR = 0.66; @@ -28,7 +28,7 @@ export function getScrollBehavior() { // Scroll tests fail if behavior is "smooth" if (process.env["NODE_ENV"] !== "production") return "instant"; - const scrollBehavior = getCachedSetting("scrollBehavior"); + const scrollBehavior = getSetting("scrollBehavior"); if (scrollBehavior === "auto") { const mediaQuery = window.matchMedia("(prefers-reduced-motion: reduce)"); diff --git a/src/content/hints/HintClass.ts b/src/content/hints/HintClass.ts index 13055fe6..08d6100b 100644 --- a/src/content/hints/HintClass.ts +++ b/src/content/hints/HintClass.ts @@ -1,27 +1,24 @@ import Color from "color"; import { debounce } from "lodash"; import { rgbaToRgb } from "../../lib/rgbaToRgb"; +import { Hint } from "../../typings/Hint"; +import { getAllSettings, getSetting } from "../settings/settingsManager"; +import { getToggles } from "../settings/toggles"; +import { createsStackingContext } from "../utils/createsStackingContext"; import { getEffectiveBackgroundColor } from "../utils/getEffectiveBackgroundColor"; import { getFirstTextNodeDescendant } from "../utils/nodeUtils"; -import { createsStackingContext } from "../utils/createsStackingContext"; -import { Hint } from "../../typings/Hint"; +import { refresh } from "../wrappers/refresh"; import { clearHintedWrapper, getWrapper, getWrapperForElement, setHintedWrapper, } from "../wrappers/wrappers"; -import { - getCachedSetting, - getCachedSettingAll, -} from "../settings/cacheSettings"; -import { refresh } from "../wrappers/refresh"; -import { getToggles } from "../settings/toggles"; import { matchesStagedSelector } from "./customSelectorsStaging"; -import { getElementToPositionHint } from "./getElementToPositionHint"; import { getAptContainer, getContextForHint } from "./getContextForHint"; +import { getCustomNudge } from "./getCustomNudge"; +import { getElementToPositionHint } from "./getElementToPositionHint"; import { popHint, pushHint } from "./hintsCache"; -import { setStyleProperties } from "./setStyleProperties"; import { cacheLayout, clearLayoutCache, @@ -31,7 +28,7 @@ import { getOffsetParent, removeFromLayoutCache, } from "./layoutCache"; -import { getCustomNudge } from "./getCustomNudge"; +import { setStyleProperties } from "./setStyleProperties"; const hintQueue: Set = new Set(); @@ -280,7 +277,7 @@ export class HintClass implements Hint { this.target = target; this.isActive = false; - this.borderWidth = getCachedSetting("hintBorderWidth"); + this.borderWidth = getSetting("hintBorderWidth"); this.shadowHost = document.createElement("div"); this.shadowHost.className = "rango-hint"; @@ -397,9 +394,9 @@ export class HintClass implements Hint { color = new Color("white"); this.borderColor = new Color("white"); } else { - const customBackgroundColor = getCachedSetting("hintBackgroundColor"); - const customFontColor = getCachedSetting("hintFontColor"); - const backgroundOpacity = getCachedSetting("hintBackgroundOpacity"); + const customBackgroundColor = getSetting("hintBackgroundColor"); + const customFontColor = getSetting("hintFontColor"); + const backgroundOpacity = getSetting("hintBackgroundOpacity"); this.firstTextNodeDescendant = getFirstTextNodeDescendant(this.target); @@ -440,7 +437,7 @@ export class HintClass implements Hint { if ( backgroundColor.contrast(color) < - getCachedSetting("hintMinimumContrastRatio") && + getSetting("hintMinimumContrastRatio") && !customFontColor ) { color = backgroundColor.isLight() @@ -448,7 +445,7 @@ export class HintClass implements Hint { : new Color("white"); } - this.borderWidth = getCachedSetting("hintBorderWidth"); + this.borderWidth = getSetting("hintBorderWidth"); this.borderColor = new Color(color).alpha(0.3); } @@ -746,7 +743,7 @@ export class HintClass implements Hint { hintBorderWidth, hintBorderRadius, hintUppercaseLetters, - } = getCachedSettingAll(); + } = getAllSettings(); this.computeColors(); @@ -777,7 +774,7 @@ export class HintClass implements Hint { clearKeyHighlight() { this.keyEmphasis = false; - this.borderWidth = getCachedSetting("hintBorderWidth"); + this.borderWidth = getSetting("hintBorderWidth"); this.updateColors(); } } diff --git a/src/content/hints/selectors.ts b/src/content/hints/selectors.ts index 46d0b74d..d8f33765 100644 --- a/src/content/hints/selectors.ts +++ b/src/content/hints/selectors.ts @@ -1,4 +1,6 @@ import { retrieve, store } from "../../common/storage"; +import { onSettingChange } from "../settings/settingsManager"; +import { refresh } from "../wrappers/refresh"; const defaultSelector = // Elements @@ -67,3 +69,8 @@ export function matchesHintableSelector(target: Element) { export function matchesExtraSelector(target: Element) { return target.matches(extraSelector); } + +onSettingChange("customSelectors", async () => { + await updateCustomSelectors(); + return refresh({ isHintable: true }); +}); diff --git a/src/content/notify/Toast.tsx b/src/content/notify/Toast.tsx index b68742f8..6f34b8c6 100644 --- a/src/content/notify/Toast.tsx +++ b/src/content/notify/Toast.tsx @@ -1,7 +1,7 @@ import "react-toastify/dist/ReactToastify.css"; import { Bounce, Flip, Slide, ToastContainer, Zoom } from "react-toastify"; import "./Toast.css"; -import { getCachedSetting } from "../settings/cacheSettings"; +import { getSetting } from "../settings/settingsManager"; const transitions = { slide: Slide, @@ -22,8 +22,8 @@ export function Toast() { newestOnTop={false} rtl={false} theme="light" - position={getCachedSetting("toastPosition")} - transition={transitions[getCachedSetting("toastTransition")]} + position={getSetting("toastPosition")} + transition={transitions[getSetting("toastTransition")]} /> ); } diff --git a/src/content/notify/notify.tsx b/src/content/notify/notify.tsx index 9dfbc1e1..ad18910d 100644 --- a/src/content/notify/notify.tsx +++ b/src/content/notify/notify.tsx @@ -1,12 +1,12 @@ -import { ToastOptions, toast } from "react-toastify"; import { createRoot } from "react-dom/client"; -import { getCachedSetting } from "../settings/cacheSettings"; -import { isCurrentTab, isMainframe } from "../setup/contentScriptContext"; +import { ToastOptions, toast } from "react-toastify"; import { retrieve } from "../../common/storage"; +import { getSetting } from "../settings/settingsManager"; +import { isCurrentTab, isMainframe } from "../setup/contentScriptContext"; import { Toast } from "./Toast"; +import { ToastIcon } from "./ToastIcon"; import { ToastMessage } from "./ToastMessage"; import { TogglesStatusMessage } from "./ToastTogglesMessage"; -import { ToastIcon } from "./ToastIcon"; let notificationAllowed = false; @@ -37,7 +37,7 @@ async function shouldNotify() { if ( !notificationAllowed || document.visibilityState !== "visible" || - !getCachedSetting("enableNotifications") || + !getSetting("enableNotifications") || !isMainframe() || !(await isCurrentTab()) ) { @@ -90,7 +90,7 @@ export async function notify(text: string, options?: ToastOptions) { export async function notifyTogglesStatus(force = false) { if ( !(await shouldNotify()) || - (!force && !getCachedSetting("notifyWhenTogglingHints")) + (!force && !getSetting("notifyWhenTogglingHints")) ) { return; } diff --git a/src/content/observe.ts b/src/content/observe.ts index f4fcffc7..2192ae19 100644 --- a/src/content/observe.ts +++ b/src/content/observe.ts @@ -1,4 +1,5 @@ -import { getCachedSetting } from "./settings/cacheSettings"; +import { notifyTogglesStatus } from "./notify/notify"; +import { getSetting, onSettingChange } from "./settings/settingsManager"; import { getToggles } from "./settings/toggles"; import { addWrappersFrom, @@ -16,7 +17,7 @@ const config = { attributes: true, childList: true, subtree: true }; export async function updateHintsEnabled() { const newEnabled = getToggles().computed; - const alwaysComputeHintables = getCachedSetting("alwaysComputeHintables"); + const alwaysComputeHintables = getSetting("alwaysComputeHintables"); // Here we assume that just one change of state takes place. That is, in the // same call to this function, either the hints have been switched or the @@ -55,7 +56,7 @@ export async function updateHintsEnabled() { export default async function observe() { enabled = getToggles().computed; - if (enabled || getCachedSetting("alwaysComputeHintables")) { + if (enabled || getSetting("alwaysComputeHintables")) { // We observe all the initial elements before any mutation if (document.body) addWrappersFrom(document.body); @@ -63,3 +64,20 @@ export default async function observe() { mutationObserver.observe(document, config); } } + +onSettingChange( + [ + "hintsToggleGlobal", + "hintsToggleHosts", + "hintsTogglePaths", + "hintsToggleTabs", + ], + async () => { + await updateHintsEnabled(); + await notifyTogglesStatus(); + } +); + +onSettingChange("alwaysComputeHintables", async () => { + await updateHintsEnabled(); +}); diff --git a/src/content/settings/cacheSettings.ts b/src/content/settings/cacheSettings.ts deleted file mode 100644 index b0a7cbfb..00000000 --- a/src/content/settings/cacheSettings.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { Settings } from "../../common/settings"; -import { retrieveSettings } from "../../common/storage"; - -let cachedSettings: Settings; - -// This function must be called when the content script loads -export async function cacheSettings() { - cachedSettings = await retrieveSettings(); -} - -export function getCachedSetting(key: T) { - return cachedSettings[key]; -} - -export function getCachedSettingAll(): Settings { - return cachedSettings; -} diff --git a/src/content/settings/settingsManager.ts b/src/content/settings/settingsManager.ts new file mode 100644 index 00000000..ebec98ec --- /dev/null +++ b/src/content/settings/settingsManager.ts @@ -0,0 +1,83 @@ +import browser from "webextension-polyfill"; +import Emittery from "emittery"; +import { defaultSettings, Settings } from "../../common/settings"; +import { retrieve, retrieveSettings } from "../../common/storage"; +import { hasMatchingKeys } from "../../lib/utils"; +import { assertDefined } from "../../typings/TypingUtils"; + +// https://github.com/microsoft/TypeScript/issues/51572#issuecomment-1319153323 +export const entries = Object.entries as ( + object: T +) => Array<[keyof T, T[keyof T]]>; + +const emitter = new Emittery(); + +let settings: Settings | undefined; + +export async function initSettingsManager() { + settings = await retrieveSettings(); + browser.storage.onChanged.addListener(async (changes) => { + // Most of the time this event fires because we are storing or retrieving + // hints stacks, so we can directly ignore it here to gain a bit of + // performance. + if (Object.keys(changes).includes("hintsStacks")) return; + + if (hasMatchingKeys(defaultSettings, changes)) { + if (document.visibilityState === "visible") { + await handleSettingChange(changes); + } else { + window.requestIdleCallback(async () => { + await handleSettingChange(changes); + }); + } + } + }); +} + +function setSetting(key: T, value: Settings[T]) { + assertDefined(settings, "Attempting to set setting before initialization."); + settings[key] = value; +} + +/** + * Synchronously get the value of a setting. It doesn't read the values from + * storage but from a cached settings object that updates every time the stored + * value changes. + */ +export function getSetting(key: T): Settings[T] { + assertDefined( + settings, + "Attempting to retrieve setting before initialization." + ); + return settings[key]; +} + +/** + * Synchronously get the value for all settings. It doesn't read the values from + * storage but from a cached settings object that updates every time the stored + * values change. + */ +export function getAllSettings() { + assertDefined( + settings, + "Attempting to retrieve settings before initialization." + ); + return settings; +} + +async function handleSettingChange( + changes: Record +) { + await Promise.all( + entries(changes).map(async ([key, change]) => { + if (change && change.oldValue !== change.newValue) { + // We need to use retrieve here to get the deserialized value. + const newValue = await retrieve(key); + setSetting(key, newValue); + await emitter.emit(key, newValue); + } + }) + ); +} + +export const onSettingChange = emitter.on.bind(emitter); diff --git a/src/content/settings/toggles.ts b/src/content/settings/toggles.ts index 08b4df2c..ad251039 100644 --- a/src/content/settings/toggles.ts +++ b/src/content/settings/toggles.ts @@ -1,5 +1,5 @@ import { getTabId } from "../setup/contentScriptContext"; -import { getCachedSettingAll } from "./cacheSettings"; +import { getAllSettings } from "./settingsManager"; let navigationToggle: boolean | undefined; @@ -13,7 +13,7 @@ export function getToggles() { hintsToggleHosts, hintsTogglePaths, hintsToggleTabs, - } = getCachedSettingAll(); + } = getAllSettings(); const tabSwitch = hintsToggleTabs.get(getTabId()); const hostSwitch = hintsToggleHosts.get(window.location.host); diff --git a/src/content/settings/watchSettingsChanges.ts b/src/content/settings/watchSettingsChanges.ts deleted file mode 100644 index 8b697717..00000000 --- a/src/content/settings/watchSettingsChanges.ts +++ /dev/null @@ -1,113 +0,0 @@ -import browser from "webextension-polyfill"; -import { defaultSettings } from "../../common/settings"; -import { hasMatchingKeys } from "../../lib/utils"; -import { updateHintsEnabled } from "../observe"; -import { - initKeyboardClicking, - stopKeyboardClicking, -} from "../actions/keyboardClicking"; -import { notify, notifyTogglesStatus } from "../notify/notify"; -import { initTitleDecoration } from "../utils/decorateTitle"; -import { refresh } from "../wrappers/refresh"; -import { updateCustomSelectors } from "../hints/selectors"; -import { cacheSettings, getCachedSetting } from "./cacheSettings"; - -async function handleSettingsChanges( - changes: Record -) { - let hasActuallyChanged = false; - - for (const key of Object.keys(changes)) { - const change = changes[key]; - if (change && change.oldValue !== change.newValue) { - hasActuallyChanged = true; - } - } - - if (!hasActuallyChanged) return; - - await cacheSettings(); - - const isToggleChange = Object.keys(changes).some((key) => - key.startsWith("hintsToggle") - ); - - if (isToggleChange) { - await updateHintsEnabled(); - await notifyTogglesStatus(); - } - - if ("alwaysComputeHintables" in changes) { - await updateHintsEnabled(); - } - - if ("keyboardClicking" in changes) { - const keyboardClicking = getCachedSetting("keyboardClicking"); - - if (keyboardClicking) { - initKeyboardClicking(); - } else { - stopKeyboardClicking(); - } - - const status = keyboardClicking ? "enabled" : "disabled"; - - await notify(`Keyboard clicking ${status}`, { - icon: status, - toastId: "keyboardToggle", - }); - - await refresh({ hintsCharacters: true }); - return; - } - - if ( - "includeSingleLetterHints" in changes || - "useNumberHints" in changes || - "hintsToExclude" in changes || - "keysToExclude" in changes - ) { - await refresh({ hintsCharacters: true }); - return; - } - - if ("customSelectors" in changes) { - await updateCustomSelectors(); - return refresh({ isHintable: true }); - } - - if ( - "urlInTitle" in changes || - "includeTabMarkers" in changes || - "uppercaseTabMarkers" in changes || - "hideTabMarkersWithGlobalHintsOff" in changes || - (Object.keys(changes).includes("hintsToggleGlobal") && - getCachedSetting("hideTabMarkersWithGlobalHintsOff")) - ) { - await initTitleDecoration(); - return; - } - - if (!isToggleChange) { - await refresh({ hintsStyle: true, hintsPosition: true }); - } -} - -export function watchSettingsChanges() { - browser.storage.onChanged.addListener(async (changes) => { - // Most of the time this event fires because we are storing or retrieving - // hints stacks, so we can directly ignore it here to gain a bit of - // performance. - if (Object.keys(changes).includes("hintsStacks")) return; - - if (hasMatchingKeys(defaultSettings, changes)) { - if (document.visibilityState === "visible") { - await handleSettingsChanges(changes); - } else { - window.requestIdleCallback(async () => { - await handleSettingsChanges(changes); - }); - } - } - }); -} diff --git a/src/content/setup/initContentScript.ts b/src/content/setup/initContentScript.ts index 5e6296c8..5c1dccbc 100644 --- a/src/content/setup/initContentScript.ts +++ b/src/content/setup/initContentScript.ts @@ -1,19 +1,17 @@ import { initKeyboardClicking } from "../actions/keyboardClicking"; import { updateCustomSelectors } from "../hints/selectors"; import observe from "../observe"; -import { cacheSettings, getCachedSetting } from "../settings/cacheSettings"; -import { watchSettingsChanges } from "../settings/watchSettingsChanges"; +import { getSetting, initSettingsManager } from "../settings/settingsManager"; import { initTitleDecoration } from "../utils/decorateTitle"; import { loadDevtoolsUtils } from "../utils/devtoolsUtils"; import { loadContentScriptContext } from "./contentScriptContext"; export async function initContentScript() { - watchSettingsChanges(); loadDevtoolsUtils(); await loadContentScriptContext(); await updateCustomSelectors(); - await cacheSettings(); + await initSettingsManager(); await initTitleDecoration(); await observe(); - if (getCachedSetting("keyboardClicking")) initKeyboardClicking(); + if (getSetting("keyboardClicking")) initKeyboardClicking(); } diff --git a/src/content/utils/decorateTitle.ts b/src/content/utils/decorateTitle.ts index 4648cbe2..f7c729cd 100644 --- a/src/content/utils/decorateTitle.ts +++ b/src/content/utils/decorateTitle.ts @@ -1,8 +1,8 @@ import browser from "webextension-polyfill"; import { throttle } from "lodash"; -import { getCachedSetting } from "../settings/cacheSettings"; import { isMainframe } from "../setup/contentScriptContext"; import { getToggles } from "../settings/toggles"; +import { getSetting, onSettingChange } from "../settings/settingsManager"; // Settings let urlInTitle: boolean; @@ -115,11 +115,11 @@ export async function initTitleDecoration() { const globalHintsOff = getToggles().global; - urlInTitle = getCachedSetting("urlInTitle"); + urlInTitle = getSetting("urlInTitle"); includeTabMarkers = - getCachedSetting("includeTabMarkers") && - !(!globalHintsOff && getCachedSetting("hideTabMarkersWithGlobalHintsOff")); - uppercaseTabMarkers = getCachedSetting("uppercaseTabMarkers"); + getSetting("includeTabMarkers") && + !(!globalHintsOff && getSetting("hideTabMarkersWithGlobalHintsOff")); + uppercaseTabMarkers = getSetting("uppercaseTabMarkers"); if ( (previousUrlInTitle && !urlInTitle) || @@ -150,3 +150,19 @@ export async function initTitleDecoration() { }); } } + +onSettingChange( + [ + "urlInTitle", + "includeTabMarkers", + "uppercaseTabMarkers", + "hideTabMarkersWithGlobalHintsOff", + ], + initTitleDecoration +); + +onSettingChange("hintsToggleGlobal", async () => { + if (getSetting("hideTabMarkersWithGlobalHintsOff")) { + await initTitleDecoration(); + } +}); diff --git a/src/content/wrappers/ElementWrapperClass.ts b/src/content/wrappers/ElementWrapperClass.ts index d8abc0e4..9d6cb187 100644 --- a/src/content/wrappers/ElementWrapperClass.ts +++ b/src/content/wrappers/ElementWrapperClass.ts @@ -6,7 +6,8 @@ import { HintClass } from "../hints/HintClass"; import { cacheHints } from "../hints/hintsCache"; import { cacheLayout, clearLayoutCache } from "../hints/layoutCache"; import { matchesCustomExclude, matchesCustomInclude } from "../hints/selectors"; -import { getCachedSetting } from "../settings/cacheSettings"; +import { setStyleProperties } from "../hints/setStyleProperties"; +import { getSetting } from "../settings/settingsManager"; import { BoundedIntersectionObserver } from "../utils/BoundedIntersectionObserver"; import { deepGetElements } from "../utils/deepGetElements"; import { @@ -20,7 +21,6 @@ import { getUserScrollableContainer } from "../utils/getUserScrollableContainer" import { isDisabled } from "../utils/isDisabled"; import { isHintable } from "../utils/isHintable"; import { isVisible } from "../utils/isVisible"; -import { setStyleProperties } from "../hints/setStyleProperties"; import { refresh } from "./refresh"; import { addWrapper, @@ -402,7 +402,7 @@ class ElementWrapperClass implements ElementWrapper { const options: IntersectionObserverInit = { root, - rootMargin: `${getCachedSetting("viewportMargin")}px`, + rootMargin: `${getSetting("viewportMargin")}px`, threshold: 0, }; diff --git a/src/content/wrappers/refresh.ts b/src/content/wrappers/refresh.ts index 28b7e986..a2c9f2e6 100644 --- a/src/content/wrappers/refresh.ts +++ b/src/content/wrappers/refresh.ts @@ -1,5 +1,6 @@ import { debounce } from "lodash"; import { cacheHints, clearHintsCache } from "../hints/hintsCache"; +import { onSettingChange } from "../settings/settingsManager"; import { getAllWrappers, getHintedWrappers } from "./wrappers"; type Options = { @@ -193,3 +194,34 @@ export async function refresh(options?: Options) { await debouncedRefresh(); } + +onSettingChange( + [ + "includeSingleLetterHints", + "useNumberHints", + "hintsToExclude", + "keysToExclude", + ], + async () => { + await refresh({ hintsCharacters: true }); + } +); + +onSettingChange( + [ + "hintUppercaseLetters", + "hintFontFamily", + "hintFontSize", + "hintWeight", + "hintBackgroundColor", + "hintBackgroundOpacity", + "hintFontColor", + "hintBorderWidth", + "hintBorderRadius", + "hintMinimumContrastRatio", + "hintBorderWidth", + ], + async () => { + await refresh({ hintsStyle: true, hintsPosition: true }); + } +); diff --git a/src/typings/TypingUtils.ts b/src/typings/TypingUtils.ts index f43f0c7c..ea8bd006 100644 --- a/src/typings/TypingUtils.ts +++ b/src/typings/TypingUtils.ts @@ -1,8 +1,11 @@ export function assertDefined( - value: T | null | undefined + value: T | null | undefined, + message?: string ): asserts value is T { if (value === null || value === undefined) { - throw new Error(`Fatal error: value must not be null/undefined.`); + throw new Error( + message ?? `Fatal error: value must not be null/undefined.` + ); } } From 16cea14aaed363f13a4e6c4c0a99a1a722c5b449 Mon Sep 17 00:00:00 2001 From: David Tejada Date: Tue, 17 Sep 2024 10:35:23 +0200 Subject: [PATCH 2/5] Rename some variables in settings manager --- src/content/settings/settingsManager.ts | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/content/settings/settingsManager.ts b/src/content/settings/settingsManager.ts index ebec98ec..c07a0bc8 100644 --- a/src/content/settings/settingsManager.ts +++ b/src/content/settings/settingsManager.ts @@ -12,10 +12,10 @@ export const entries = Object.entries as ( const emitter = new Emittery(); -let settings: Settings | undefined; +let settingsCache: Settings | undefined; export async function initSettingsManager() { - settings = await retrieveSettings(); + settingsCache = await retrieveSettings(); browser.storage.onChanged.addListener(async (changes) => { // Most of the time this event fires because we are storing or retrieving // hints stacks, so we can directly ignore it here to gain a bit of @@ -34,9 +34,12 @@ export async function initSettingsManager() { }); } -function setSetting(key: T, value: Settings[T]) { - assertDefined(settings, "Attempting to set setting before initialization."); - settings[key] = value; +function cacheSetting(key: T, value: Settings[T]) { + assertDefined( + settingsCache, + "Attempting to set setting before initialization." + ); + settingsCache[key] = value; } /** @@ -46,10 +49,10 @@ function setSetting(key: T, value: Settings[T]) { */ export function getSetting(key: T): Settings[T] { assertDefined( - settings, + settingsCache, "Attempting to retrieve setting before initialization." ); - return settings[key]; + return settingsCache[key]; } /** @@ -59,10 +62,10 @@ export function getSetting(key: T): Settings[T] { */ export function getAllSettings() { assertDefined( - settings, + settingsCache, "Attempting to retrieve settings before initialization." ); - return settings; + return settingsCache; } async function handleSettingChange( @@ -73,7 +76,7 @@ async function handleSettingChange( if (change && change.oldValue !== change.newValue) { // We need to use retrieve here to get the deserialized value. const newValue = await retrieve(key); - setSetting(key, newValue); + cacheSetting(key, newValue); await emitter.emit(key, newValue); } }) From d79eb7b407ad92a420c20f11873d5012f3e2cead Mon Sep 17 00:00:00 2001 From: David Tejada Date: Wed, 18 Sep 2024 10:36:44 +0200 Subject: [PATCH 3/5] Replace uses of `store` with `getSetting` for retrieving settings in content scripts --- src/content/actions/customScrollPositions.ts | 7 ++++--- src/content/actions/references.ts | 5 +++-- src/content/hints/selectors.ts | 6 +++--- src/content/notify/notify.tsx | 5 ++--- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/content/actions/customScrollPositions.ts b/src/content/actions/customScrollPositions.ts index 88789897..bbe7db55 100644 --- a/src/content/actions/customScrollPositions.ts +++ b/src/content/actions/customScrollPositions.ts @@ -1,6 +1,7 @@ import Fuse from "fuse.js"; -import { retrieve, store } from "../../common/storage"; +import { store } from "../../common/storage"; import { notify } from "../notify/notify"; +import { getSetting } from "../settings/settingsManager"; import { getMainScrollable, getScrollBehavior } from "./scroll"; export async function storeScrollPosition(name: string) { @@ -13,7 +14,7 @@ export async function storeScrollPosition(name: string) { const { scrollTop } = scrollContainer; - const scrollPositions = await retrieve("customScrollPositions"); + const scrollPositions = getSetting("customScrollPositions"); const scrollPositionsForCurrentPage = scrollPositions.get(window.location.origin + window.location.pathname) ?? new Map(); @@ -32,7 +33,7 @@ export async function storeScrollPosition(name: string) { export async function scrollToPosition(name: string) { const scrollContainer = getMainScrollable("vertical"); - const scrollPositions = await retrieve("customScrollPositions"); + const scrollPositions = getSetting("customScrollPositions"); const scrollPositionsForCurrentPage = scrollPositions.get(window.location.origin + window.location.pathname) ?? new Map(); diff --git a/src/content/actions/references.ts b/src/content/actions/references.ts index e7276b46..4d98e5b6 100644 --- a/src/content/actions/references.ts +++ b/src/content/actions/references.ts @@ -1,12 +1,13 @@ import browser from "webextension-polyfill"; import { getCssSelector } from "css-selector-generator"; -import { retrieve, store } from "../../common/storage"; +import { store } from "../../common/storage"; import { ElementWrapper } from "../../typings/ElementWrapper"; import { showTooltip } from "../hints/showTooltip"; import { getOrCreateWrapper } from "../wrappers/ElementWrapperClass"; import { getHostPattern } from "../../common/utils"; import { getActiveElement } from "../utils/domUtils"; import { getWrapperForElement } from "../wrappers/wrappers"; +import { getSetting } from "../settings/settingsManager"; function getWrapperFromUniqueSelector(selector: string) { const element = document.querySelector(selector); @@ -17,7 +18,7 @@ function getWrapperFromUniqueSelector(selector: string) { export async function getReferences() { const hostPattern = getHostPattern(window.location.href); - const references = await retrieve("references"); + const references = getSetting("references"); const hostReferences = references.get(hostPattern) ?? new Map(); diff --git a/src/content/hints/selectors.ts b/src/content/hints/selectors.ts index d8f33765..0f90e6a4 100644 --- a/src/content/hints/selectors.ts +++ b/src/content/hints/selectors.ts @@ -1,5 +1,5 @@ -import { retrieve, store } from "../../common/storage"; -import { onSettingChange } from "../settings/settingsManager"; +import { store } from "../../common/storage"; +import { getSetting, onSettingChange } from "../settings/settingsManager"; import { refresh } from "../wrappers/refresh"; const defaultSelector = @@ -23,7 +23,7 @@ let excludeSelectorAll = ""; * used when checking if an element should be hinted. */ export async function updateCustomSelectors() { - const customSelectors = await retrieve("customSelectors"); + const customSelectors = getSetting("customSelectors"); // This is stored when the extension first runs, so it shouldn't be undefined. // But it is undefined when running tests. This way we also make extra sure. diff --git a/src/content/notify/notify.tsx b/src/content/notify/notify.tsx index ad18910d..9b9dcbf5 100644 --- a/src/content/notify/notify.tsx +++ b/src/content/notify/notify.tsx @@ -1,6 +1,5 @@ import { createRoot } from "react-dom/client"; import { ToastOptions, toast } from "react-toastify"; -import { retrieve } from "../../common/storage"; import { getSetting } from "../settings/settingsManager"; import { isCurrentTab, isMainframe } from "../setup/contentScriptContext"; import { Toast } from "./Toast"; @@ -52,7 +51,7 @@ export async function notify(text: string, options?: ToastOptions) { renderToast(); - const autoClose = await retrieve("toastDuration"); + const autoClose = getSetting("toastDuration"); options = Object.assign({ autoClose }, options); @@ -97,7 +96,7 @@ export async function notifyTogglesStatus(force = false) { renderToast(); - const autoClose = await retrieve("toastDuration"); + const autoClose = getSetting("toastDuration"); if (toast.isActive("toggles")) { toast.update("toggles"); From 6477f5cd626496baa54787d0ee7340e7ce5f0437 Mon Sep 17 00:00:00 2001 From: David Tejada Date: Wed, 18 Sep 2024 10:46:53 +0200 Subject: [PATCH 4/5] Remove unused exports --- src/content/actions/keyboardClicking.ts | 2 +- src/content/settings/settingsManager.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/content/actions/keyboardClicking.ts b/src/content/actions/keyboardClicking.ts index 30934861..d1a0e1d1 100644 --- a/src/content/actions/keyboardClicking.ts +++ b/src/content/actions/keyboardClicking.ts @@ -102,7 +102,7 @@ export function initKeyboardClicking() { window.addEventListener("keydown", keydownHandler, true); } -export function stopKeyboardClicking() { +function stopKeyboardClicking() { window.removeEventListener("keydown", keydownHandler, true); } diff --git a/src/content/settings/settingsManager.ts b/src/content/settings/settingsManager.ts index c07a0bc8..ba6d2c4f 100644 --- a/src/content/settings/settingsManager.ts +++ b/src/content/settings/settingsManager.ts @@ -6,7 +6,7 @@ import { hasMatchingKeys } from "../../lib/utils"; import { assertDefined } from "../../typings/TypingUtils"; // https://github.com/microsoft/TypeScript/issues/51572#issuecomment-1319153323 -export const entries = Object.entries as ( +const entries = Object.entries as ( object: T ) => Array<[keyof T, T[keyof T]]>; From 34e3abec51283d3b4ba9ed601d0f86835230c9b8 Mon Sep 17 00:00:00 2001 From: David Tejada Date: Wed, 18 Sep 2024 11:02:38 +0200 Subject: [PATCH 5/5] Fix settings manager not being ready when updating custom selectors --- src/content/setup/initContentScript.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/content/setup/initContentScript.ts b/src/content/setup/initContentScript.ts index 5c1dccbc..2a0e3cf6 100644 --- a/src/content/setup/initContentScript.ts +++ b/src/content/setup/initContentScript.ts @@ -9,8 +9,8 @@ import { loadContentScriptContext } from "./contentScriptContext"; export async function initContentScript() { loadDevtoolsUtils(); await loadContentScriptContext(); - await updateCustomSelectors(); await initSettingsManager(); + await updateCustomSelectors(); await initTitleDecoration(); await observe(); if (getSetting("keyboardClicking")) initKeyboardClicking();