mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
[ui] Make "Share on Nostr" a working button
The share menu's "Share on Nostr" entry was never clickable, no matter who was
logged in -- it rendered as inert, dimmed text.
VideoModal initialized `shareNostrAuthState` to { isLoggedIn: false, hasSigner:
false } and nothing ever wrote to it. createVideoShareMenuPanel gates the item
on `isLoggedIn && hasSigner`, so it was always disabled, and the click
delegation in bindShareMenuActions() bails on `button.disabled` -- so the whole
share-to-Nostr flow behind it was unreachable.
The renderer and the publish path were both already correct; only the gate was
missing:
- VideoModal.setShareNostrAuthState() records the state and closes an already
open share menu when it changes, so a menu opened before login does not keep
showing the stale disabled item. It is a no-op when nothing changed, to avoid
slamming the menu shut on every sync.
- syncAuthUiState() computes the gate from isUserLoggedIn() plus a
signing-capable active signer, mirroring ShareNostrController.handleShare():
a signer without signEvent would only produce an error toast on click.
- syncAuthUiState() was only ever called at bootstrap, so logging in
mid-session left the gate closed. It now self-installs a bitvid:auth-changed
listener on first run (bound once, re-entrancy safe) rather than adding a
call site in js/app.js, which is at its file-size cap.
Verified in a browser that the rendered item is disabled when logged out or
signer-less and enabled + click-dispatching with both; the new tests fail 5/9
with the gate push removed.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
1 parent
b84558e928
commit
d141c316a9
3 files changed
+242
-1
No files matched your search
+43
-1
@@ -9,6 +9,7 @@
|
||||
* Methods use `this` which is bound to the Application instance.
|
||||
*/
|
||||
import { SHORT_TIMEOUT_MS, DEBOUNCE_DELAY_MS } from "../constants.js";
|
||||
import { getActiveSigner } from "../nostrClientRegistry.js";
|
||||
|
||||
export function createUiCoordinator(deps) {
|
||||
const {
|
||||
@@ -187,11 +188,52 @@ export function createUiCoordinator(deps) {
|
||||
},
|
||||
|
||||
syncAuthUiState() {
|
||||
if (this.isUserLoggedIn()) {
|
||||
// Auth-dependent chrome has to follow login/logout/account switches, but
|
||||
// syncAuthUiState() was only ever called at bootstrap — so everything it
|
||||
// gates (notably the share menu's "Share on Nostr" item) kept its
|
||||
// logged-out state for the entire session. Self-install a listener on the
|
||||
// first run rather than adding a call site in js/app.js, which is at its
|
||||
// file-size cap.
|
||||
if (
|
||||
!this.__authUiSyncBound &&
|
||||
typeof window !== "undefined" &&
|
||||
typeof window?.addEventListener === "function"
|
||||
) {
|
||||
this.__authUiSyncBound = true;
|
||||
window.addEventListener("bitvid:auth-changed", () => {
|
||||
try {
|
||||
this.syncAuthUiState();
|
||||
} catch (error) {
|
||||
devLogger.warn("[Application] Auth UI resync failed:", error);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
const loggedIn = this.isUserLoggedIn();
|
||||
if (loggedIn) {
|
||||
this.applyAuthenticatedUiState();
|
||||
} else {
|
||||
this.applyLoggedOutUiState();
|
||||
}
|
||||
|
||||
// Gate the share menu's "Share on Nostr" item. Without this the modal's
|
||||
// shareNostrAuthState stays at its initial `false` forever and the item
|
||||
// renders permanently disabled — indistinguishable from plain text.
|
||||
if (typeof this.videoModal?.setShareNostrAuthState === "function") {
|
||||
let signer = null;
|
||||
try {
|
||||
signer = getActiveSigner();
|
||||
} catch (error) {
|
||||
devLogger.warn("[Application] Failed to read active signer:", error);
|
||||
}
|
||||
this.videoModal.setShareNostrAuthState({
|
||||
isLoggedIn: loggedIn,
|
||||
// Mirrors ShareNostrController.handleShare(): a signer that cannot
|
||||
// sign is no signer at all, and enabling the item would only produce
|
||||
// an error toast on click.
|
||||
hasSigner: !!signer && typeof signer.signEvent === "function",
|
||||
});
|
||||
}
|
||||
},
|
||||
|
||||
refreshChromeElements() {
|
||||
|
||||
@@ -3453,6 +3453,32 @@ export class VideoModal {
|
||||
// Delegated click handler for the share popover's menu-action buttons
|
||||
// (Copy URL / Copy Magnet / Copy CDN / Share on Nostr). The forced-source
|
||||
// test links self-wire their own copy, so they are skipped here.
|
||||
/**
|
||||
* Update the login/signer gate for the share menu's "Share on Nostr" item.
|
||||
*
|
||||
* This state was initialized to `false` and never written, so
|
||||
* `createVideoShareMenuPanel` always rendered the item disabled — the entry
|
||||
* looked like inert text no matter who was logged in, and the click
|
||||
* delegation in bindShareMenuActions() bails on `button.disabled`. The app
|
||||
* pushes real state here from syncAuthUiState().
|
||||
*/
|
||||
setShareNostrAuthState({ isLoggedIn = false, hasSigner = false } = {}) {
|
||||
const next = { isLoggedIn: !!isLoggedIn, hasSigner: !!hasSigner };
|
||||
const changed =
|
||||
next.isLoggedIn !== this.shareNostrAuthState.isLoggedIn ||
|
||||
next.hasSigner !== this.shareNostrAuthState.hasSigner;
|
||||
this.shareNostrAuthState = next;
|
||||
|
||||
// The popover renders its panel on open, so a menu that is already open
|
||||
// would keep showing the stale (disabled) item. Close it so the next open
|
||||
// renders against current state.
|
||||
if (changed) {
|
||||
this.modalSharePopover?.close?.();
|
||||
}
|
||||
|
||||
return changed;
|
||||
}
|
||||
|
||||
bindShareMenuActions(panel) {
|
||||
if (!panel || typeof panel.addEventListener !== "function") {
|
||||
return;
|
||||
|
||||
@@ -0,0 +1,173 @@
|
||||
// SCN-share-nostr-gate
|
||||
//
|
||||
// The share menu's "Share on Nostr" item is gated on `isLoggedIn && hasSigner`
|
||||
// (js/ui/components/videoMenuRenderers.js). VideoModal initialized
|
||||
// `shareNostrAuthState` to {false, false} and NOTHING ever wrote to it, so the
|
||||
// item rendered permanently disabled — it looked like inert text, and the click
|
||||
// delegation in bindShareMenuActions() bails on `button.disabled`.
|
||||
//
|
||||
// Two halves have to hold for the button to work:
|
||||
// 1. VideoModal.setShareNostrAuthState() records the state.
|
||||
// 2. syncAuthUiState() pushes real auth state in, and re-runs on auth change
|
||||
// (it previously ran only once at bootstrap).
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { createUiCoordinator } from "../js/app/uiCoordinator.js";
|
||||
import { VideoModal } from "../js/ui/components/VideoModal.js";
|
||||
import {
|
||||
clearActiveSigner,
|
||||
registerSigner,
|
||||
setActiveSigner,
|
||||
} from "../js/nostrClientRegistry.js";
|
||||
|
||||
const PUBKEY = "b".repeat(64);
|
||||
|
||||
// --- VideoModal setter ----------------------------------------------------
|
||||
|
||||
function makeModal() {
|
||||
const modal = Object.create(VideoModal.prototype);
|
||||
modal.shareNostrAuthState = { isLoggedIn: false, hasSigner: false };
|
||||
modal.closedPopover = 0;
|
||||
modal.modalSharePopover = {
|
||||
close: () => {
|
||||
modal.closedPopover += 1;
|
||||
},
|
||||
};
|
||||
return modal;
|
||||
}
|
||||
|
||||
test("setShareNostrAuthState records the gate", () => {
|
||||
const modal = makeModal();
|
||||
assert.equal(modal.setShareNostrAuthState({ isLoggedIn: true, hasSigner: true }), true);
|
||||
assert.deepEqual(modal.shareNostrAuthState, { isLoggedIn: true, hasSigner: true });
|
||||
});
|
||||
|
||||
test("setShareNostrAuthState is idempotent and only closes a stale menu once", () => {
|
||||
const modal = makeModal();
|
||||
modal.setShareNostrAuthState({ isLoggedIn: true, hasSigner: true });
|
||||
assert.equal(modal.closedPopover, 1, "open menu must re-render against new state");
|
||||
|
||||
// No change -> no churn. Closing the popover on every no-op sync would slam
|
||||
// the menu shut under the user.
|
||||
assert.equal(modal.setShareNostrAuthState({ isLoggedIn: true, hasSigner: true }), false);
|
||||
assert.equal(modal.closedPopover, 1);
|
||||
});
|
||||
|
||||
test("setShareNostrAuthState coerces and defaults safely", () => {
|
||||
const modal = makeModal();
|
||||
modal.setShareNostrAuthState({ isLoggedIn: "yes", hasSigner: 1 });
|
||||
assert.deepEqual(modal.shareNostrAuthState, { isLoggedIn: true, hasSigner: true });
|
||||
|
||||
modal.setShareNostrAuthState();
|
||||
assert.deepEqual(modal.shareNostrAuthState, { isLoggedIn: false, hasSigner: false });
|
||||
});
|
||||
|
||||
test("setShareNostrAuthState tolerates no popover", () => {
|
||||
const modal = makeModal();
|
||||
modal.modalSharePopover = null;
|
||||
assert.doesNotThrow(() =>
|
||||
modal.setShareNostrAuthState({ isLoggedIn: true, hasSigner: true })
|
||||
);
|
||||
});
|
||||
|
||||
// --- syncAuthUiState wiring ----------------------------------------------
|
||||
|
||||
function makeApp({ loggedIn }) {
|
||||
const coordinator = createUiCoordinator({
|
||||
devLogger: { warn() {}, error() {}, log() {} },
|
||||
});
|
||||
const pushed = [];
|
||||
const app = {
|
||||
...coordinator,
|
||||
isUserLoggedIn: () => loggedIn,
|
||||
// Stub the DOM-touching branches; this test is about the share gate.
|
||||
applyAuthenticatedUiState() {},
|
||||
applyLoggedOutUiState() {},
|
||||
videoModal: {
|
||||
setShareNostrAuthState(state) {
|
||||
pushed.push({ ...state });
|
||||
},
|
||||
},
|
||||
};
|
||||
return { app, pushed };
|
||||
}
|
||||
|
||||
test("syncAuthUiState pushes the gate into the video modal", () => {
|
||||
const { app, pushed } = makeApp({ loggedIn: true });
|
||||
app.syncAuthUiState();
|
||||
assert.equal(pushed.length, 1);
|
||||
assert.equal(pushed[0].isLoggedIn, true);
|
||||
});
|
||||
|
||||
test("logged out propagates as a closed gate", () => {
|
||||
const { app, pushed } = makeApp({ loggedIn: false });
|
||||
app.syncAuthUiState();
|
||||
assert.deepEqual(pushed[0], { isLoggedIn: false, hasSigner: false });
|
||||
});
|
||||
|
||||
test("hasSigner reflects a registered, signing-capable signer", () => {
|
||||
clearActiveSigner();
|
||||
const { app, pushed } = makeApp({ loggedIn: true });
|
||||
|
||||
app.syncAuthUiState();
|
||||
assert.equal(pushed[0].hasSigner, false, "no signer registered yet");
|
||||
|
||||
registerSigner(PUBKEY, { pubkey: PUBKEY, signEvent: async (e) => e });
|
||||
setActiveSigner(PUBKEY);
|
||||
app.syncAuthUiState();
|
||||
assert.equal(pushed[1].hasSigner, true);
|
||||
|
||||
clearActiveSigner();
|
||||
app.syncAuthUiState();
|
||||
assert.equal(pushed[2].hasSigner, false, "logout must close the gate again");
|
||||
});
|
||||
|
||||
test("a signer that cannot sign does not open the gate", () => {
|
||||
// Mirrors ShareNostrController.handleShare(): enabling the item for a signer
|
||||
// without signEvent would only produce an error toast on click.
|
||||
clearActiveSigner();
|
||||
registerSigner(PUBKEY, { pubkey: PUBKEY });
|
||||
setActiveSigner(PUBKEY);
|
||||
|
||||
const { app, pushed } = makeApp({ loggedIn: true });
|
||||
app.syncAuthUiState();
|
||||
assert.equal(pushed[0].hasSigner, false);
|
||||
clearActiveSigner();
|
||||
});
|
||||
|
||||
test("syncAuthUiState re-runs on bitvid:auth-changed, and binds only once", () => {
|
||||
// The original bug's second half: the gate was computed once at bootstrap and
|
||||
// never again, so logging in mid-session left the button disabled.
|
||||
const listeners = [];
|
||||
const originalWindow = globalThis.window;
|
||||
globalThis.window = {
|
||||
addEventListener(type, handler) {
|
||||
listeners.push({ type, handler });
|
||||
},
|
||||
};
|
||||
|
||||
try {
|
||||
const { app, pushed } = makeApp({ loggedIn: false });
|
||||
app.syncAuthUiState();
|
||||
assert.equal(pushed.length, 1);
|
||||
|
||||
const bound = listeners.filter((l) => l.type === "bitvid:auth-changed");
|
||||
assert.equal(bound.length, 1, "listener installed on first sync");
|
||||
|
||||
bound[0].handler();
|
||||
assert.equal(pushed.length, 2, "auth change re-syncs the gate");
|
||||
|
||||
// Re-entrancy: the resync must not stack a new listener every time.
|
||||
assert.equal(
|
||||
listeners.filter((l) => l.type === "bitvid:auth-changed").length,
|
||||
1
|
||||
);
|
||||
} finally {
|
||||
if (originalWindow === undefined) {
|
||||
delete globalThis.window;
|
||||
} else {
|
||||
globalThis.window = originalWindow;
|
||||
}
|
||||
}
|
||||
});
|
||||
Reference in new issue
Block a user