Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spotlight cleanup #5015

Merged
merged 12 commits into from
Nov 1, 2024
15 changes: 10 additions & 5 deletions app/packages/spotlight/src/createScrollReader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@ export default function createScrollReader(
let timeout: ReturnType<typeof setTimeout>;
let zooming = false;

element.addEventListener("scroll", () => {
const scroll = () => {
scrolling = true;
});
};
element.addEventListener("scroll", scroll);

element.addEventListener("scrollend", () => {
const scrollEnd = () => {
scrolling = false;
requestAnimationFrame(() => render(zooming, true));
});
};

element.addEventListener("scrollend", scrollEnd);

const updateScrollStatus = () => {
const threshold = getScrollSpeedThreshold();
Expand Down Expand Up @@ -68,7 +71,9 @@ export default function createScrollReader(
return {
destroy: () => {
destroyed = true;
element.removeEventListener("scroll", scroll);
element.removeEventListener("scrollend", scrollEnd);
},
zooming: () => zooming
zooming: () => zooming,
};
}
10 changes: 5 additions & 5 deletions app/packages/spotlight/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
SCROLLBAR_WIDTH,
TWO,
ZERO,
ZOOMING_COEFFICIENT
ZOOMING_COEFFICIENT,
} from "./constants";
import createScrollReader from "./createScrollReader";
import { Load, RowChange } from "./events";
Expand Down Expand Up @@ -108,8 +108,8 @@ export default class Spotlight<K, V> extends EventTarget {
return;
}

this.#backward?.remove();
this.#forward?.remove();
this.#backward?.destroy();
this.#forward?.destroy();
this.#element?.classList.remove(styles.spotlightLoaded);
this.#element?.remove();
this.#scrollReader?.destroy();
Expand Down Expand Up @@ -204,7 +204,7 @@ export default class Spotlight<K, V> extends EventTarget {
const backward = this.#forward;
this.#forward = section;
this.#forward.attach(this.#element);
this.#backward.remove();
this.#backward.destroy();
benjaminpkane marked this conversation as resolved.
Show resolved Hide resolved
this.#backward = backward;
offset = before - this.#containerHeight + this.#config.spacing;
}
Expand Down Expand Up @@ -272,7 +272,7 @@ export default class Spotlight<K, V> extends EventTarget {
this.#backward = result.section;
this.#backward.attach(this.#element);

this.#forward.remove();
this.#forward.destroy();
this.#forward = forward;
}

Expand Down
46 changes: 34 additions & 12 deletions app/packages/spotlight/src/row.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export default class Row<K, V> {
#from: number;
#hidden: boolean;

readonly #aborter: AbortController = new AbortController();
readonly #config: SpotlightConfig<K, V>;
readonly #dangle?: boolean;
readonly #container: HTMLDivElement = create(DIV);
Expand Down Expand Up @@ -47,7 +48,7 @@ export default class Row<K, V> {
element.style.top = pixels(ZERO);

if (config.onItemClick) {
element.addEventListener("click", (event) => {
const handler = (event) => {
if (event.metaKey || event.shiftKey) {
return;
}
Expand All @@ -59,18 +60,13 @@ export default class Row<K, V> {
item,
iter,
});
};

element.addEventListener("click", handler, {
signal: this.#aborter.signal,
});
element.addEventListener("contextmenu", (event) => {
if (event.metaKey || event.shiftKey) {
return;
}
event.preventDefault();
focus(item.id);
config.onItemClick({
event,
item,
iter,
});
element.addEventListener("contextmenu", handler, {
signal: this.#aborter.signal,
});
}

Expand Down Expand Up @@ -123,6 +119,11 @@ export default class Row<K, V> {
return this.#row[this.#row.length - ONE].item.id;
}

destroy() {
this.#destroyItems();
this.#aborter.abort();
}

has(item: string) {
for (const i of this.#row) {
if (i.item.id.description === item) {
Expand All @@ -138,6 +139,7 @@ export default class Row<K, V> {
}

this.#container.remove();
this.#destroyItems();
}

show(
Expand Down Expand Up @@ -225,4 +227,24 @@ export default class Row<K, V> {
const set = new Set(this.#row.map(({ item }) => item.aspectRatio));
return set.size === ONE ? this.#row[ZERO].item.aspectRatio : null;
}

#destroyItems() {
const destroy = this.#config.destroy;
if (!destroy) {
return;
}

const errors = [];
for (const item of this.#row) {
try {
destroy(item.item.id);
} catch (e) {
errors.push(e);
}
}

if (errors.length > 0) {
console.error("Errors occurred during row destruction:", errors);
}
}
}
11 changes: 6 additions & 5 deletions app/packages/spotlight/src/section.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ export default class Section<K, V> {
: element.appendChild(this.#section);
}

destroy() {
this.#section.remove();
for (const row of this.#rows) row.destroy();
this.#rows = [];
}
benjaminpkane marked this conversation as resolved.
Show resolved Hide resolved

find(item: string): Row<K, V> | null {
for (const row of this.#rows) {
if (row.has(item)) {
Expand All @@ -118,11 +124,6 @@ export default class Section<K, V> {
return null;
}

remove() {
this.#section.remove();
this.#rows = [];
}

render({
config,
target,
Expand Down
3 changes: 2 additions & 1 deletion app/packages/spotlight/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ export type Request<K, V> = (key: K) => Promise<{
}>;

export interface SpotlightConfig<K, V> {
get: Get<K, V>;
at?: At;
destroy?: (id: ID) => void;
get: Get<K, V>;
key: K;
offset?: number;
onItemClick?: ItemClick<K, V>;
Expand Down
Loading