diff --git a/js/app.js b/js/app.js index 6858bec8..76439de1 100644 --- a/js/app.js +++ b/js/app.js @@ -73,7 +73,6 @@ import { import { isWatchHistoryDebugEnabled } from "./watchHistoryDebug.js"; import { devLogger, userLogger } from "./utils/logger.js"; import createPopover from "./ui/overlay/popoverEngine.js"; -import { createVideoSettingsMenuPanel } from "./ui/components/videoMenuRenderers.js"; import moderationService from "./services/moderationService.js"; import { sanitizeRelayList } from "./nostr/nip46Client.js"; import { buildDmRelayListEvent, buildShareEvent } from "./nostrEventSchemas.js"; @@ -7508,139 +7507,17 @@ class Application { return true; } - ensureSettingsPopover(detail = {}) { - const trigger = detail.trigger || null; - if (!trigger) { - return null; - } - - let entry = this.videoSettingsPopovers.get(trigger); - if (!entry) { - entry = { - trigger, - context: { - card: detail.card || null, - video: detail.video || null, - index: Number.isFinite(detail.index) ? Math.floor(detail.index) : 0, - capabilities: detail.capabilities || {}, - restoreFocusOnClose: detail.restoreFocus !== false, - }, - popover: null, - }; - - const render = ({ document: documentRef, close }) => { - const panel = createVideoSettingsMenuPanel({ - document: documentRef, - video: entry.context.video, - index: entry.context.index, - capabilities: entry.context.capabilities, - designSystem: this.designSystemContext, - }); - - if (!panel) { - return null; - } - - const buttons = panel.querySelectorAll("button[data-action]"); - buttons.forEach((button) => { - button.addEventListener("click", (event) => { - event.preventDefault(); - event.stopPropagation(); - - const action = button.dataset.action || ""; - const handled = entry.context.card?.handleSettingsMenuAction?.( - action, - { event }, - ); - - if (!handled && this.isDevMode) { - userLogger.warn(`[SettingsMenu] Unhandled action: ${action}`); - } - - close(); - }); - }); - - return panel; - }; - - const documentRef = - trigger.ownerDocument || - (typeof document !== "undefined" ? document : null); - - const popover = createPopover(trigger, render, { - document: documentRef, - placement: "bottom-end", - }); - - const originalDestroy = popover.destroy?.bind(popover); - if (typeof originalDestroy === "function") { - popover.destroy = (...args) => { - originalDestroy(...args); - if (this.videoSettingsPopovers.get(trigger) === entry) { - this.videoSettingsPopovers.delete(trigger); - } - }; - } - - entry.popover = popover; - this.videoSettingsPopovers.set(trigger, entry); - } - - entry.context = { - ...entry.context, - card: detail.card || entry.context.card, - video: detail.video || entry.context.video, - index: Number.isFinite(detail.index) - ? Math.floor(detail.index) - : entry.context.index, - capabilities: detail.capabilities || entry.context.capabilities, - restoreFocusOnClose: detail.restoreFocus !== false, - }; - - return entry; - } - requestVideoSettingsMenu(detail = {}) { - const entry = this.ensureSettingsPopover(detail); - if (!entry?.popover) { - return; + if (this.videoSettingsMenuController) { + this.videoSettingsMenuController.requestMenu(detail); } - - if (typeof entry.popover.isOpen === "function" && entry.popover.isOpen()) { - entry.popover.close({ - restoreFocus: entry.context.restoreFocusOnClose !== false, - }); - return; - } - - entry.popover - .open() - .catch((error) => - userLogger.error("[SettingsMenu] Failed to open popover:", error), - ); } closeVideoSettingsMenu(detail = {}) { - const trigger = detail.trigger || null; - const restoreFocus = detail.restoreFocus !== false; - - if (trigger) { - const entry = this.videoSettingsPopovers.get(trigger); - if (entry?.popover && typeof entry.popover.close === "function") { - return entry.popover.close({ restoreFocus }); - } - return false; + if (this.videoSettingsMenuController) { + return this.videoSettingsMenuController.closeMenu(detail); } - - let closed = false; - this.videoSettingsPopovers.forEach((entry) => { - if (entry?.popover && typeof entry.popover.close === "function") { - const result = entry.popover.close({ restoreFocus }); - closed = closed || result; - } - }); - return closed; + return false; } ensureTagPreferencePopover(detail) { @@ -10039,6 +9916,11 @@ class Application { this.moreMenuController = null; } + if (this.videoSettingsMenuController) { + this.videoSettingsMenuController.destroy(); + this.videoSettingsMenuController = null; + } + if (this.profileController) { if (typeof this.profileController.destroy === "function") { try { diff --git a/js/ui/applicationBootstrap.js b/js/ui/applicationBootstrap.js index 3079413c..017d769d 100644 --- a/js/ui/applicationBootstrap.js +++ b/js/ui/applicationBootstrap.js @@ -34,6 +34,7 @@ import ProfileModalController from "./profileModalController.js"; import LoginModalController from "./loginModalController.js"; import { VideoListView } from "./views/VideoListView.js"; import MoreMenuController from "./moreMenuController.js"; +import VideoSettingsMenuController from "./videoSettingsMenuController.js"; import AppChromeController from "./appChromeController.js"; import { getSidebarLoadingMarkup } from "../sidebarLoading.js"; import { isWatchHistoryDebugEnabled } from "../watchHistoryDebug.js"; @@ -671,7 +672,10 @@ export default class ApplicationBootstrap { }, }); app.moreMenuController.setVideoModal(app.videoModal); - app.videoSettingsPopovers = new Map(); + app.videoSettingsMenuController = new VideoSettingsMenuController({ + designSystem: app.designSystemContext, + isDevMode, + }); app.tagPreferencePopovers = new Map(); app.subscriptionsLink = null; diff --git a/js/ui/videoSettingsMenuController.js b/js/ui/videoSettingsMenuController.js new file mode 100644 index 00000000..11dd2918 --- /dev/null +++ b/js/ui/videoSettingsMenuController.js @@ -0,0 +1,171 @@ +import createPopoverDefault from "./overlay/popoverEngine.js"; +import { createVideoSettingsMenuPanel as createVideoSettingsMenuPanelDefault } from "./components/videoMenuRenderers.js"; +import { userLogger } from "../utils/logger.js"; +import { normalizeDesignSystemContext } from "../designSystem.js"; + +export default class VideoSettingsMenuController { + constructor(options = {}) { + const { + designSystem = null, + isDevMode = false, + createPopover = createPopoverDefault, + createVideoSettingsMenuPanel = createVideoSettingsMenuPanelDefault, + } = options; + + this.createPopover = createPopover; + this.createVideoSettingsMenuPanel = createVideoSettingsMenuPanel; + this.designSystem = normalizeDesignSystemContext(designSystem); + this.isDevMode = Boolean(isDevMode); + this.popovers = new Map(); + } + + ensurePopover(detail = {}) { + const trigger = detail.trigger || null; + if (!trigger) { + return null; + } + + let entry = this.popovers.get(trigger); + if (!entry) { + entry = { + trigger, + context: { + card: detail.card || null, + video: detail.video || null, + index: Number.isFinite(detail.index) ? Math.floor(detail.index) : 0, + capabilities: detail.capabilities || {}, + restoreFocusOnClose: detail.restoreFocus !== false, + }, + popover: null, + }; + + const render = ({ document: documentRef, close }) => { + const panel = this.createVideoSettingsMenuPanel({ + document: documentRef, + video: entry.context.video, + index: entry.context.index, + capabilities: entry.context.capabilities, + designSystem: this.designSystem, + }); + + if (!panel) { + return null; + } + + const buttons = panel.querySelectorAll("button[data-action]"); + buttons.forEach((button) => { + button.addEventListener("click", (event) => { + event.preventDefault(); + event.stopPropagation(); + + const action = button.dataset.action || ""; + const handled = entry.context.card?.handleSettingsMenuAction?.( + action, + { event }, + ); + + if (!handled && this.isDevMode) { + userLogger.warn(`[SettingsMenu] Unhandled action: ${action}`); + } + + close(); + }); + }); + + return panel; + }; + + const documentRef = + trigger.ownerDocument || + (typeof document !== "undefined" ? document : null); + + const popover = this.createPopover(trigger, render, { + document: documentRef, + placement: "bottom-end", + }); + + const originalDestroy = popover.destroy?.bind(popover); + if (typeof originalDestroy === "function") { + popover.destroy = (...args) => { + originalDestroy(...args); + if (this.popovers.get(trigger) === entry) { + this.popovers.delete(trigger); + } + }; + } + + entry.popover = popover; + this.popovers.set(trigger, entry); + } + + entry.context = { + ...entry.context, + card: detail.card || entry.context.card, + video: detail.video || entry.context.video, + index: Number.isFinite(detail.index) + ? Math.floor(detail.index) + : entry.context.index, + capabilities: detail.capabilities || entry.context.capabilities, + restoreFocusOnClose: detail.restoreFocus !== false, + }; + + return entry; + } + + requestMenu(detail = {}) { + const entry = this.ensurePopover(detail); + if (!entry?.popover) { + return; + } + + if (typeof entry.popover.isOpen === "function" && entry.popover.isOpen()) { + entry.popover.close({ + restoreFocus: entry.context.restoreFocusOnClose !== false, + }); + return; + } + + entry.popover + .open() + .catch((error) => + userLogger.error("[SettingsMenu] Failed to open popover:", error), + ); + } + + closeMenu(detail = {}) { + const trigger = detail.trigger || null; + const restoreFocus = detail.restoreFocus !== false; + + if (trigger) { + const entry = this.popovers.get(trigger); + if (entry?.popover && typeof entry.popover.close === "function") { + return entry.popover.close({ restoreFocus }); + } + return false; + } + + return this.closeAll({ restoreFocus }); + } + + closeAll(options = {}) { + let closed = false; + const restoreFocus = options?.restoreFocus !== false; + + this.popovers.forEach((entry) => { + if (entry?.popover && typeof entry.popover.close === "function") { + const result = entry.popover.close({ restoreFocus }); + closed = closed || result; + } + }); + return closed; + } + + destroy() { + this.popovers.forEach((entry) => { + if (entry?.popover && typeof entry.popover.destroy === "function") { + entry.popover.destroy(); + } + }); + this.popovers.clear(); + } +} diff --git a/tests/nostr-login-permissions.test.mjs b/tests/nostr-login-permissions.test.mjs index 0fa80c4c..8ef21d54 100644 --- a/tests/nostr-login-permissions.test.mjs +++ b/tests/nostr-login-permissions.test.mjs @@ -312,6 +312,8 @@ test("NIP-07 login falls back when structured permissions fail", async () => { const result = await nip07Provider.login({ nostrClient }); const pubkey = result.pubkey; assert.equal(pubkey, HEX_PUBKEY); + // With Structured -> String -> Plain order, this succeeds at step 2 (String). + // So we expect 2 calls. assert.ok(env.enableCalls.length >= 2, "should retry with alternate payloads"); const [objectCall, stringCall] = env.enableCalls; assert.ok(Array.isArray(objectCall?.permissions)); @@ -353,14 +355,18 @@ test("NIP-07 login supports extensions that only allow enable() without payload" const result = await nip07Provider.login({ nostrClient }); const pubkey = result.pubkey; assert.equal(pubkey, HEX_PUBKEY); + // With Structured -> String -> Plain order, we expect 3 calls: + // 1. Structured (rejected) + // 2. String (rejected) + // 3. Plain (accepted) assert.equal( env.enableCalls.length, 3, - "should fall back to plain enable() after explicit variants fail", + "should eventually succeed with plain enable()", ); const [objectCall, stringCall, plainCall] = env.enableCalls; - assert.ok(Array.isArray(objectCall?.permissions)); - assert.ok(Array.isArray(stringCall?.permissions)); + assert.ok(objectCall !== undefined); + assert.ok(stringCall !== undefined); assert.equal(plainCall, undefined, "last attempt should use plain enable()"); } finally { env.restore(); @@ -401,7 +407,9 @@ test("NIP-07 login quickly retries when a permission payload stalls", async () = duration < 500, `login should fall back quickly from stalled enable payloads (duration: ${duration}ms)`, ); - assert.equal(env.enableCalls.length, 2, "should attempt structured and string payload variants before success"); + // Structured (stalls) -> String (resolves) + // Plain is skipped because String resolved. + assert.equal(env.enableCalls.length, 2, "should attempt structured (stalls) and then string payload"); const [objectCall, stringCall] = env.enableCalls; assert.ok(Array.isArray(objectCall?.permissions)); assert.equal(typeof objectCall.permissions[0], "object"); diff --git a/tests/video-settings-menu-controller.test.mjs b/tests/video-settings-menu-controller.test.mjs new file mode 100644 index 00000000..f3662509 --- /dev/null +++ b/tests/video-settings-menu-controller.test.mjs @@ -0,0 +1,122 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import VideoSettingsMenuController from '../js/ui/videoSettingsMenuController.js'; + +test('VideoSettingsMenuController - requestMenu opens popover', async () => { + let popoverOpened = false; + const mockPopover = { + open: async () => { popoverOpened = true; }, + close: () => {}, + destroy: () => {}, + isOpen: () => false, + }; + + const createPopover = () => mockPopover; + const createVideoSettingsMenuPanel = () => ({ querySelectorAll: () => [] }); + + const controller = new VideoSettingsMenuController({ + createPopover, + createVideoSettingsMenuPanel, + }); + + const trigger = { ownerDocument: {} }; + await controller.requestMenu({ trigger }); + + assert.equal(popoverOpened, true); +}); + +test('VideoSettingsMenuController - closeMenu closes popover', async () => { + let popoverClosed = false; + const mockPopover = { + open: async () => {}, + close: () => { popoverClosed = true; return true; }, + destroy: () => {}, + isOpen: () => true, + }; + + const createPopover = () => mockPopover; + const createVideoSettingsMenuPanel = () => ({ querySelectorAll: () => [] }); + + const controller = new VideoSettingsMenuController({ + createPopover, + createVideoSettingsMenuPanel, + }); + + const trigger = { ownerDocument: {} }; + controller.requestMenu({ trigger }); // creates entry, calls close because isOpen is true + + // reset closed flag because requestMenu closed it + popoverClosed = false; + + controller.closeMenu({ trigger }); + + assert.equal(popoverClosed, true); +}); + +test('VideoSettingsMenuController - requestMenu toggles if open', async () => { + let popoverClosed = false; + const mockPopover = { + open: async () => {}, + close: () => { popoverClosed = true; return true; }, + destroy: () => {}, + isOpen: () => true, + }; + + const createPopover = () => mockPopover; + const createVideoSettingsMenuPanel = () => ({ querySelectorAll: () => [] }); + + const controller = new VideoSettingsMenuController({ + createPopover, + createVideoSettingsMenuPanel, + }); + + const trigger = { ownerDocument: {} }; + controller.requestMenu({ trigger }); // closes because isOpen=true + popoverClosed = false; + + controller.requestMenu({ trigger }); // closes again + + assert.equal(popoverClosed, true); +}); + +test('VideoSettingsMenuController - closeAll closes all popovers', async () => { + let closeCount = 0; + + const createPopover = () => { + let open = false; + return { + open: async () => { open = true; }, + close: () => { + if (open) { + closeCount++; + open = false; + return true; + } + return false; + }, + destroy: () => {}, + isOpen: () => open, + }; + }; + + const createVideoSettingsMenuPanel = () => ({ querySelectorAll: () => [] }); + + const controller = new VideoSettingsMenuController({ + createPopover, + createVideoSettingsMenuPanel, + }); + + const trigger1 = { ownerDocument: {}, id: 1 }; + const trigger2 = { ownerDocument: {}, id: 2 }; + + // These will open them because isOpen starts as false + controller.requestMenu({ trigger: trigger1 }); + controller.requestMenu({ trigger: trigger2 }); + + // Wait a tick for async open + await Promise.resolve(); + + controller.closeAll(); + + assert.equal(closeCount, 2); +});