fix: address review feedback (active-doc, settings broadcast, locked-size CSS)

This commit is contained in:
黄鸿峰 2026-04-30 11:05:05 +08:00
parent ca048675a8
commit e0577b825a
9 changed files with 147 additions and 53 deletions

View file

@ -93,7 +93,7 @@ export const en = {
"notice.importNoTable": "Clipboard does not contain a recognizable table.",
"notice.importDone": "Imported table from clipboard ({source}).",
"notice.importFailed": "Could not read clipboard. Try again or paste manually.",
"notice.copiedToClipboard": "Table markdown copied to clipboard.",
"notice.copiedToClipboard": "Table copied to clipboard as Markdown.",
"notice.copyFailed": "Could not write to clipboard.",
// Settings

View file

@ -21,9 +21,15 @@ export function setLanguage(choice: LangChoice): void {
}
function detectObsidianLocale(): string {
// Obsidian exposes the locale via window.moment.locale()
// Obsidian exposes the locale via activeWindow.moment.locale()
// (`activeWindow` is the popout-window-aware global Obsidian provides;
// referencing the bare `window` global would trip obsidianmd's
// `prefer-active-doc` lint rule).
// Fall back to browser language.
const w = typeof window !== "undefined" ? (window as unknown as { moment?: { locale?: () => string } }) : undefined;
const w =
typeof activeWindow !== "undefined"
? (activeWindow as unknown as { moment?: { locale?: () => string } })
: undefined;
if (w?.moment?.locale) {
try {
return w.moment.locale() ?? "en";

View file

@ -110,7 +110,7 @@ export default class TableMasterPlugin extends Plugin {
// active leaf.
this.registerEvent(
this.app.workspace.on("active-leaf-change", () => {
document.dispatchEvent(new CustomEvent("table-master:settings-changed"));
this.broadcastSettingsChanged();
}),
);
@ -129,9 +129,19 @@ export default class TableMasterPlugin extends Plugin {
// CodeMirror views when a plugin unloads, so any floating toolbars that
// were attached to a markdown view's wrapper may otherwise leak. Remove
// every DOM node we tagged on construction.
document
.querySelectorAll<HTMLElement>('[data-tm-floating-toolbar="1"]')
.forEach((el) => el.remove());
// We scan every document that hosts a markdown leaf (main + popout
// windows) — the bare `document` global only sees the main window, which
// would leak any toolbar that lives in a popout. obsidianmd's
// `prefer-active-doc` rule forbids that bare reference too.
const seen = new Set<Document>();
this.app.workspace.iterateAllLeaves((leaf) => {
const doc = leaf.view?.containerEl?.ownerDocument;
if (!doc || seen.has(doc)) return;
seen.add(doc);
doc
.querySelectorAll<HTMLElement>('[data-tm-floating-toolbar="1"]')
.forEach((el) => el.remove());
});
}
async loadSettings(): Promise<void> {
@ -142,6 +152,25 @@ export default class TableMasterPlugin extends Plugin {
await this.saveData(this.settings);
}
/**
* Dispatch the `table-master:settings-changed` event into every document
* that hosts a markdown view. We can't rely on the global `document` (the
* obsidianmd lint rule forbids that bare reference, and it would also miss
* popout windows entirely), so we walk the workspace's leaves and emit the
* event on each unique `ownerDocument`. Each floating-toolbar instance
* subscribes to its own `view.dom.ownerDocument`, so this guarantees the
* broadcast reaches main + popout windows alike.
*/
broadcastSettingsChanged(): void {
const seen = new Set<Document>();
this.app.workspace.iterateAllLeaves((leaf) => {
const doc = leaf.view?.containerEl?.ownerDocument;
if (!doc || seen.has(doc)) return;
seen.add(doc);
doc.dispatchEvent(new CustomEvent("table-master:settings-changed"));
});
}
private registerCommands(): void {
const editorCmd = (id: string, name: string, fn: (ctx: actions.ActionContext) => void) => {
this.addCommand({

View file

@ -33,14 +33,19 @@ export function buildLivePreviewExt() {
destroy() {
if (this.timer != null) {
window.clearTimeout(this.timer);
// Use the editor's own window so the timer is cleared in popout
// windows too. Bare `window` would only resolve to the main window
// and trip obsidianmd's `prefer-active-doc` lint rule.
const win = this.view.dom.ownerDocument.defaultView ?? activeWindow;
win.clearTimeout(this.timer);
this.timer = null;
}
}
private schedule() {
if (this.timer != null) return;
this.timer = window.setTimeout(() => {
const win = this.view.dom.ownerDocument.defaultView ?? activeWindow;
this.timer = win.setTimeout(() => {
this.timer = null;
this.run();
}, 50);

View file

@ -33,16 +33,22 @@ export async function applyModelToTable(
table.className = className;
table.classList.add("tm-rendered");
// Build new nodes through the table's own ownerDocument so the markup
// also lives in the right document tree when the markdown view is in a
// popout window. obsidianmd's `prefer-active-doc` lint rule additionally
// forbids referencing the bare `document` global.
const doc = table.ownerDocument;
if (model.caption) {
const cap = document.createElement("caption");
const cap = doc.createElement("caption");
cap.textContent = model.caption.text;
table.appendChild(cap);
}
if (model.headerRows > 0) {
const thead = document.createElement("thead");
const thead = doc.createElement("thead");
for (let r = 0; r < model.headerRows; r++) {
thead.appendChild(rowToTr(model, r, "th", opts));
thead.appendChild(rowToTr(doc, model, r, "th", opts));
}
table.appendChild(thead);
}
@ -52,10 +58,10 @@ export async function applyModelToTable(
let tbody: HTMLTableSectionElement | null = null;
for (let r = model.headerRows; r < model.rows.length; r++) {
if (!tbody || breaks.has(r)) {
tbody = document.createElement("tbody");
tbody = doc.createElement("tbody");
table.appendChild(tbody);
}
tbody.appendChild(rowToTr(model, r, "td", opts));
tbody.appendChild(rowToTr(doc, model, r, "td", opts));
}
}
@ -67,16 +73,17 @@ export async function applyModelToTable(
}
function rowToTr(
doc: Document,
model: TableModel,
r: number,
tag: "th" | "td",
opts: ApplyOptions,
): HTMLTableRowElement {
const tr = document.createElement("tr");
const tr = doc.createElement("tr");
for (let c = 0; c < model.cols; c++) {
const cell = model.rows[r][c];
if (!cell.isAnchor) continue;
const td = document.createElement(tag);
const td = doc.createElement(tag);
if (cell.rowspan > 1) td.setAttribute("rowspan", String(cell.rowspan));
if (cell.colspan > 1) td.setAttribute("colspan", String(cell.colspan));
const align = model.aligns[c];
@ -89,8 +96,8 @@ function rowToTr(
// takes over.
const pieces = cell.raw.split("\n");
pieces.forEach((piece, idx) => {
if (idx > 0) td.appendChild(document.createElement("br"));
td.appendChild(document.createTextNode(piece));
if (idx > 0) td.appendChild(doc.createElement("br"));
td.appendChild(doc.createTextNode(piece));
});
}
tr.appendChild(td);

View file

@ -20,17 +20,6 @@ export type FloatingToolbarPosition =
| "follow-mouse"
| "top-left";
/**
* Broadcast a "settings changed" event so every active floating-toolbar
* instance can re-place itself immediately, without the user having to also
* click in the editor to trigger a CM6 update.
*/
function notifyToolbarPositionChange(): void {
if (typeof document !== "undefined") {
document.dispatchEvent(new CustomEvent("table-master:settings-changed"));
}
}
export interface TableMasterSettings {
outputFormat: OutputFormat;
showFloatingToolbar: boolean;
@ -100,7 +89,7 @@ export class TableMasterSettingTab extends PluginSettingTab {
tg.onChange(async (v) => {
this.plugin.settings.showFloatingToolbar = v;
await this.plugin.saveSettings();
notifyToolbarPositionChange();
this.plugin.broadcastSettingsChanged();
});
});
@ -120,7 +109,7 @@ export class TableMasterSettingTab extends PluginSettingTab {
dd.onChange(async (v) => {
this.plugin.settings.floatingToolbarPosition = v as FloatingToolbarPosition;
await this.plugin.saveSettings();
notifyToolbarPositionChange();
this.plugin.broadcastSettingsChanged();
});
});

View file

@ -60,7 +60,13 @@ export function buildFloatingToolbarExt(host: ToolbarHost) {
constructor(view: EditorView) {
this.view = view;
this.dom = document.createElement("div");
// Use the editor's own ownerDocument (and its defaultView) instead of
// the global `document` / `window` so the toolbar keeps working when
// the markdown view lives inside a popout window. obsidianmd's
// `prefer-active-doc` lint rule explicitly forbids referencing the
// bare `document` / `window` globals.
const doc = view.dom.ownerDocument;
this.dom = doc.createElement("div");
this.dom.className = "tm-floating-toolbar";
// Tagged so `TableMasterPlugin.onunload` can sweep up any toolbars
// whose CM6 destroy() hook didn't fire (Obsidian doesn't always
@ -80,12 +86,14 @@ export function buildFloatingToolbarExt(host: ToolbarHost) {
// that ancestor instead of the viewport, which is why the user saw
// the toolbar systematically offset to the bottom-right. <body> is
// never inside such a transform, so fixed positioning is reliable.
document.body.appendChild(this.dom);
// We mount on the editor's own document body so popout windows get
// their own toolbar instead of one stranded in the main window.
doc.body.appendChild(this.dom);
this.render();
// Subscribe to settings broadcasts so the toolbar reacts to a position
// change without requiring the user to also click in the editor.
this.settingsListener = () => this.refresh(this.view);
document.addEventListener("table-master:settings-changed", this.settingsListener);
doc.addEventListener("table-master:settings-changed", this.settingsListener);
// Track scroll on the editor's scroll container so the toolbar can
// re-anchor itself if a future placement mode wants viewport-tied
// coordinates. CM6's `viewportChanged` only fires when the rendered
@ -100,7 +108,10 @@ export function buildFloatingToolbarExt(host: ToolbarHost) {
});
};
view.scrollDOM.addEventListener("scroll", this.scrollListener, { passive: true });
window.addEventListener("scroll", this.scrollListener, { passive: true, capture: true });
// Listen on the view's own window so popout windows receive their
// own scroll events; the main `window` global wouldn't fire for them.
const win = doc.defaultView ?? activeWindow;
win.addEventListener("scroll", this.scrollListener, { passive: true, capture: true });
// Install the on-click mousedown handler eagerly. It MUST exist before
// the user's first click on a table — otherwise that click happens
// while no listener is attached and the toolbar misses it. The handler
@ -152,22 +163,26 @@ export function buildFloatingToolbarExt(host: ToolbarHost) {
}
destroy() {
// Mirror the constructor's choice of doc/win so add/remove pair on
// the same EventTarget even after the view has been detached.
const doc = this.view.dom.ownerDocument;
const win = doc.defaultView ?? activeWindow;
this.dom.remove();
if (this.mouseListener) {
this.view.dom.removeEventListener("mousemove", this.mouseListener);
this.mouseListener = null;
}
if (this.mouseDownListener) {
document.removeEventListener("mousedown", this.mouseDownListener, true);
doc.removeEventListener("mousedown", this.mouseDownListener, true);
this.mouseDownListener = null;
}
if (this.settingsListener) {
document.removeEventListener("table-master:settings-changed", this.settingsListener);
doc.removeEventListener("table-master:settings-changed", this.settingsListener);
this.settingsListener = null;
}
if (this.scrollListener) {
this.view.scrollDOM.removeEventListener("scroll", this.scrollListener);
window.removeEventListener("scroll", this.scrollListener, true);
win.removeEventListener("scroll", this.scrollListener, true);
this.scrollListener = null;
}
if (this.raf != null) {
@ -357,13 +372,15 @@ export function buildFloatingToolbarExt(host: ToolbarHost) {
this.placeTopLeft(this.view);
}
};
document.addEventListener("mousedown", fn, true);
// Listen on the view's own document so the on-click handler still
// fires when the editor lives inside a popout window.
this.view.dom.ownerDocument.addEventListener("mousedown", fn, true);
this.mouseDownListener = fn;
}
private removeMouseDownListener() {
if (!this.mouseDownListener) return;
document.removeEventListener("mousedown", this.mouseDownListener, true);
this.view.dom.ownerDocument.removeEventListener("mousedown", this.mouseDownListener, true);
this.mouseDownListener = null;
}
@ -399,8 +416,11 @@ export function buildFloatingToolbarExt(host: ToolbarHost) {
this.ensureVisible();
const myWidth = this.dom.offsetWidth || 200;
const myHeight = this.dom.offsetHeight || 80;
const maxLeft = window.innerWidth - myWidth - 4;
const maxTop = window.innerHeight - myHeight - 4;
// Clamp against the editor's own window so popout-window toolbars
// get the right viewport extents.
const win = this.view.dom.ownerDocument.defaultView ?? activeWindow;
const maxLeft = win.innerWidth - myWidth - 4;
const maxTop = win.innerHeight - myHeight - 4;
const left = Math.min(Math.max(this.clientX, 4), Math.max(maxLeft, 4));
const top = Math.min(Math.max(this.clientY, 4), Math.max(maxTop, 4));
this.dom.style.top = `${top}px`;
@ -470,16 +490,31 @@ export function buildFloatingToolbarExt(host: ToolbarHost) {
toggleBtn.setAttribute("aria-label", t("tip.collapseToolbar"));
toggleBtn.title = t("tip.collapseToolbar");
appendSvgIcon(toggleBtn, Icons.chevronLeft);
// The collapse animation needs the content's natural size pinned for
// the duration of the transition (otherwise the flex child snaps
// straight to 0 with no animation). Rather than writing
// element.style.width/height directly — which obsidianmd's lint
// forbids — we publish the measured size as CSS custom properties via
// `setCssProps` and toggle a `.is-size-locked` class that consumes
// them (see styles.css).
const lockContentSize = () => {
content.style.width = `${content.offsetWidth}px`;
content.style.height = `${content.offsetHeight}px`;
content.dataset.tmWidth = String(content.offsetWidth);
content.dataset.tmHeight = String(content.offsetHeight);
const w = content.offsetWidth;
const h = content.offsetHeight;
content.setCssProps({
"--tm-locked-width": `${w}px`,
"--tm-locked-height": `${h}px`,
});
content.dataset.tmWidth = String(w);
content.dataset.tmHeight = String(h);
content.classList.add("is-size-locked");
};
const unlockContentSize = () => {
if (this.dom.classList.contains("is-collapsed")) return;
content.style.width = "";
content.style.height = "";
content.classList.remove("is-size-locked");
content.setCssProps({
"--tm-locked-width": "",
"--tm-locked-height": "",
});
};
toggleBtn.addEventListener("click", (e) => {
e.preventDefault();
@ -492,10 +527,18 @@ export function buildFloatingToolbarExt(host: ToolbarHost) {
this.placeTopLeft(this.view);
this.clickedInTable = false;
} else {
if (content.dataset.tmWidth) content.style.width = `${content.dataset.tmWidth}px`;
if (content.dataset.tmHeight) content.style.height = `${content.dataset.tmHeight}px`;
if (content.dataset.tmWidth && content.dataset.tmHeight) {
content.setCssProps({
"--tm-locked-width": `${content.dataset.tmWidth}px`,
"--tm-locked-height": `${content.dataset.tmHeight}px`,
});
content.classList.add("is-size-locked");
}
this.dom.classList.remove("is-collapsed");
setTimeout(unlockContentSize, 350);
// Use the view's own window timer so the cleanup still fires
// when the editor is in a popout window.
const win = this.view.dom.ownerDocument.defaultView ?? activeWindow;
win.setTimeout(unlockContentSize, 350);
}
});
}

View file

@ -51,7 +51,10 @@ export class GridEditorModal extends Modal {
}
onClose(): void {
document.removeEventListener("mouseup", this.boundMouseUp);
// Use the modal's own ownerDocument so the listener is removed from the
// same document we attached it on (popout-window aware). obsidianmd's
// `prefer-active-doc` lint rule forbids the bare `document` global.
this.contentEl.ownerDocument.removeEventListener("mouseup", this.boundMouseUp);
this.contentEl.empty();
}
@ -117,7 +120,9 @@ export class GridEditorModal extends Modal {
if (this.selectedCells.has(this.key(r, c))) td.addClass("tm-selected");
}
}
document.addEventListener("mouseup", this.boundMouseUp);
// Pair with the removeEventListener in onClose; both use the modal's
// ownerDocument so the listener is correctly scoped to the same window.
this.contentEl.ownerDocument.addEventListener("mouseup", this.boundMouseUp);
}
private renderActions(parent: HTMLElement): void {

View file

@ -46,6 +46,16 @@
transition: width 0.3s ease, opacity 0.25s ease;
}
/* While the collapse animation is in flight, the JS toggle adds
* `is-size-locked` and writes the measured size into CSS custom properties
* (`--tm-locked-width` / `--tm-locked-height`) via setCssProps. The class is
* removed (and the variables cleared) once the transition settles, so the
* content can reflow naturally on subsequent layout changes. */
.tm-floating-toolbar .tm-toolbar-content.is-size-locked {
width: var(--tm-locked-width);
height: var(--tm-locked-height);
}
.tm-floating-toolbar.is-collapsed .tm-toolbar-content {
width: 0 !important;
opacity: 0;