Skip to content

Commit

Permalink
Refactor settings watching to reduce coupling (#312)
Browse files Browse the repository at this point in the history
* Refactor settings watcher in content scripts to reduce decoupling

* Rename some variables in settings manager

* Replace uses of `store` with `getSetting` for retrieving settings in content scripts

* Remove unused exports

* Fix settings manager not being ready when updating custom selectors
  • Loading branch information
david-tejada authored Sep 18, 2024
1 parent 2f31ecf commit 88b04fe
Show file tree
Hide file tree
Showing 21 changed files with 287 additions and 201 deletions.
55 changes: 46 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 4 additions & 3 deletions src/content/actions/customScrollPositions.ts
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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<string, number>();
Expand All @@ -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<string, number>();
Expand Down
22 changes: 21 additions & 1 deletion src/content/actions/keyboardClicking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof setTimeout>;
Expand Down Expand Up @@ -99,6 +102,23 @@ export function initKeyboardClicking() {
window.addEventListener("keydown", keydownHandler, true);
}

export function stopKeyboardClicking() {
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 });
});
5 changes: 3 additions & 2 deletions src/content/actions/references.ts
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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<string, string>();

Expand Down
4 changes: 2 additions & 2 deletions src/content/actions/runActionOnTextMatchedElement.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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) => ({
Expand Down
4 changes: 2 additions & 2 deletions src/content/actions/scroll.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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)");
Expand Down
35 changes: 16 additions & 19 deletions src/content/hints/HintClass.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -31,7 +28,7 @@ import {
getOffsetParent,
removeFromLayoutCache,
} from "./layoutCache";
import { getCustomNudge } from "./getCustomNudge";
import { setStyleProperties } from "./setStyleProperties";

const hintQueue: Set<HintClass> = new Set();

Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -440,15 +437,15 @@ export class HintClass implements Hint {

if (
backgroundColor.contrast(color) <
getCachedSetting("hintMinimumContrastRatio") &&
getSetting("hintMinimumContrastRatio") &&
!customFontColor
) {
color = backgroundColor.isLight()
? new Color("black")
: new Color("white");
}

this.borderWidth = getCachedSetting("hintBorderWidth");
this.borderWidth = getSetting("hintBorderWidth");
this.borderColor = new Color(color).alpha(0.3);
}

Expand Down Expand Up @@ -746,7 +743,7 @@ export class HintClass implements Hint {
hintBorderWidth,
hintBorderRadius,
hintUppercaseLetters,
} = getCachedSettingAll();
} = getAllSettings();

this.computeColors();

Expand Down Expand Up @@ -777,7 +774,7 @@ export class HintClass implements Hint {

clearKeyHighlight() {
this.keyEmphasis = false;
this.borderWidth = getCachedSetting("hintBorderWidth");
this.borderWidth = getSetting("hintBorderWidth");
this.updateColors();
}
}
11 changes: 9 additions & 2 deletions src/content/hints/selectors.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { retrieve, store } from "../../common/storage";
import { store } from "../../common/storage";
import { getSetting, onSettingChange } from "../settings/settingsManager";
import { refresh } from "../wrappers/refresh";

const defaultSelector =
// Elements
Expand All @@ -21,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.
Expand Down Expand Up @@ -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 });
});
6 changes: 3 additions & 3 deletions src/content/notify/Toast.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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")]}
/>
);
}
Loading

0 comments on commit 88b04fe

Please sign in to comment.