From dc9e4c32001b85547cd233c82396a725ea92dcd2 Mon Sep 17 00:00:00 2001 From: Olivier Date: Mon, 7 Sep 2026 20:03:45 -0400 Subject: [PATCH] fix: week-number middle-click crash and 'No icon' never clearing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Middle-clicking a calendar week number threw ReferenceError: monday is not defined (shipped in v1.5.1) — the variable was declared inside the click closure and read from the sibling mousedown handler. Hoist it to weekStart. - promptIcon guarded on if (iconName), so the picker's 'No icon' link submitting '' was silently dropped and the old icon stayed. '' is a valid value; always apply it. - renderCalendar scanned vault.getFiles() per day cell (up to 42 full vault scans per render); use the O(1) plugin.hasNoteForDate(). - Collapse ~180 lines of triplicated drag listeners into one attachBookmarkDragHandlers(); replace (el as any).__dropAbove/__dropInto stashing with a typed WeakMap; drop the getLeaf(x as any) casts. - dragManager.dragFile was called twice per dragstart; call it once behind a single narrowly-typed accessor. - Cache the Lucide catalog per session instead of refetching the CDN on every picker open; normalize IconSuggestModal to the file's const/arrow style. --- src/views/waypoint-view.ts | 399 ++++++++++++++++--------------------- 1 file changed, 174 insertions(+), 225 deletions(-) diff --git a/src/views/waypoint-view.ts b/src/views/waypoint-view.ts index 6f8418e..10b3159 100644 --- a/src/views/waypoint-view.ts +++ b/src/views/waypoint-view.ts @@ -12,6 +12,7 @@ import { Notice, TFile, moment, + type PaneType, } from 'obsidian'; import type WaypointPlugin from 'src/main'; import { getMonthGrid } from 'src/utils/date-utils'; @@ -19,6 +20,18 @@ import { BookmarkItem } from 'src/models/bookmark'; export const WAYPOINT_VIEW_TYPE = 'waypoint-view'; +/** Obsidian's internal drag manager — undocumented, so it has no public typings. */ +interface DragManager { + dragFile(event: DragEvent, file: TFile): unknown; + onDragStart(event: DragEvent, draggable: unknown): void; +} + +function getDragManager(app: App): DragManager { + // `dragManager` is an internal Obsidian API, absent from the public typings. + const internal = app as unknown as { dragManager: DragManager }; + return internal.dragManager; +} + export class WaypointView extends ItemView { private plugin: WaypointPlugin; @@ -72,6 +85,7 @@ export class WaypointView extends ItemView { private currentDisplayMonth: number = moment().month(); // 0-indexed private currentDisplayYear: number = moment().year(); private dragId: string | null = null; + private dropZones = new WeakMap(); private recentFilesFilter: string | null = null; // null = show all, else filter by type private renderCalendar(): void { @@ -97,7 +111,7 @@ export class WaypointView extends ItemView { qEl.addEventListener('mousedown', (event: MouseEvent) => { if (event.button === 1) { event.preventDefault(); - this.plugin.openPeriodNoteInLeaf('quarter', displayDate, this.app.workspace.getLeaf('tab')); + this.plugin.openPeriodNote('quarter', displayDate, this.app.workspace.getLeaf('tab')); } }); @@ -109,7 +123,7 @@ export class WaypointView extends ItemView { mEl.addEventListener('mousedown', (event: MouseEvent) => { if (event.button === 1) { event.preventDefault(); - this.plugin.openPeriodNoteInLeaf('month', displayDate, this.app.workspace.getLeaf('tab')); + this.plugin.openPeriodNote('month', displayDate, this.app.workspace.getLeaf('tab')); } }); @@ -118,10 +132,10 @@ export class WaypointView extends ItemView { yEl.addEventListener('click', () => { this.plugin.openPeriodNote('year', displayDate); }); - yEl.addEventListener('mousedown', (event: MouseEvent) => { + yEl.addEventListener('mousedown', (event: MouseEvent) => { if (event.button === 1) { event.preventDefault(); - this.plugin.openPeriodNoteInLeaf('year', displayDate, this.app.workspace.getLeaf('tab')); + this.plugin.openPeriodNote('year', displayDate, this.app.workspace.getLeaf('tab')); } }); @@ -172,15 +186,14 @@ export class WaypointView extends ItemView { // Week number cell const wnCell = row.createEl('td', { cls: 'waypoint-weeknum' }); wnCell.setText(String(week.weekNumber)); + const weekStart = week.days[0].date; wnCell.addEventListener('click', () => { - const monday = week.days[0].date; - this.plugin.openPeriodNote('week', monday); + this.plugin.openPeriodNote('week', weekStart); }); wnCell.addEventListener('mousedown', (event: MouseEvent) => { if (event.button === 1) { event.preventDefault(); - const file = this.app.workspace.getLeaf('tab'); - this.plugin.openPeriodNoteInLeaf('week', monday, file); + this.plugin.openPeriodNote('week', weekStart, this.app.workspace.getLeaf('tab')); } }); @@ -197,10 +210,7 @@ export class WaypointView extends ItemView { if (this.plugin.settings.calendar.showNoteIndicators) { const dateStr = day.date.format('YYYY-MM-DD'); - const hasNote = this.plugin.app.vault.getFiles().some( - f => f.extension === 'md' && f.basename === dateStr, - ); - if (hasNote) { + if (this.plugin.hasNoteForDate(dateStr)) { cell.addClass('has-note'); } } @@ -211,8 +221,7 @@ export class WaypointView extends ItemView { cell.addEventListener('mousedown', (event: MouseEvent) => { if (event.button === 1) { event.preventDefault(); - const file = this.app.workspace.getLeaf('tab'); - this.plugin.openPeriodNoteInLeaf('day', day.date, file); + this.plugin.openPeriodNote('day', day.date, this.app.workspace.getLeaf('tab')); } }); } @@ -284,7 +293,7 @@ export class WaypointView extends ItemView { // Type pills const sortedTypes = configuredTags.length > 0 ? Object.entries(typeCounts) // preserve configured order - : Object.entries(typeCounts).sort(([,a], [,b]) => b - a); // sort by count + : Object.entries(typeCounts).sort((a, b) => b[1] - a[1]); // sort by count for (const [type, count] of sortedTypes) { const pill = filterBar.createSpan({ cls: `waypoint-recent-pill${this.recentFilesFilter === type ? ' is-active' : ''}` }); pill.setText(`${type} ${count}`); @@ -339,8 +348,9 @@ export class WaypointView extends ItemView { navFileTitle.addEventListener('dragstart', (event: DragEvent) => { const tfile = this.app.metadataCache.getFirstLinkpathDest(file.path, ''); if (tfile) { - (this.app as any).dragManager.dragFile(event, tfile); - (this.app as any).dragManager.onDragStart(event, (this.app as any).dragManager.dragFile(event, tfile)); + const dragManager = getDragManager(this.app); + const draggable = dragManager.dragFile(event, tfile); + dragManager.onDragStart(event, draggable); } }); @@ -398,10 +408,10 @@ export class WaypointView extends ItemView { } } - private focusFile(file: { path: string; basename: string }, newLeaf: boolean | string | 'split'): void { + private focusFile(file: { path: string; basename: string }, newLeaf: PaneType | boolean): void { const targetFile = this.app.vault.getFiles().find(f => f.path === file.path); if (targetFile) { - const leaf = this.app.workspace.getLeaf(newLeaf as any); + const leaf = this.app.workspace.getLeaf(newLeaf); leaf.openFile(targetFile); } else { new Notice('Cannot find file'); @@ -499,45 +509,7 @@ export class WaypointView extends ItemView { rowEl.style.cursor = 'grab'; // Drag events - rowEl.addEventListener('dragstart', (e) => { - this.dragId = item.id; - e.dataTransfer!.effectAllowed = 'move'; - e.dataTransfer!.setData('text/plain', item.id); - rowEl.addClass('waypoint-bm-dragging'); - }); - rowEl.addEventListener('dragend', () => { - this.dragId = null; - container.querySelectorAll('.waypoint-bm-dragging, .waypoint-bm-drop-line, .waypoint-bm-drop-below, .waypoint-bm-drop-into').forEach(el => { - el.removeClass('waypoint-bm-dragging'); - el.removeClass('waypoint-bm-drop-line'); - el.removeClass('waypoint-bm-drop-below'); - }); - }); - rowEl.addEventListener('dragenter', (e) => { - e.preventDefault(); - if (!this.dragId || this.dragId === item.id) return; - this.showDropIndicator(rowEl, e, false); - }); - rowEl.addEventListener('dragover', (e) => { - e.preventDefault(); - if (!this.dragId || this.dragId === item.id) return; - this.showDropIndicator(rowEl, e, false); - }); - rowEl.addEventListener('dragleave', () => { - rowEl.removeClass('waypoint-bm-drop-line'); - rowEl.removeClass('waypoint-bm-drop-below'); - rowEl.removeClass('waypoint-bm-drop-into'); - }); - rowEl.addEventListener('drop', (e) => { - e.preventDefault(); - this.dragId = null; - rowEl.removeClass('waypoint-bm-drop-line'); - rowEl.removeClass('waypoint-bm-drop-below'); - const draggedId = e.dataTransfer?.getData('text/plain'); - if (!draggedId || draggedId === item.id) return; - const dropAbove = (rowEl as any).__dropAbove; - this.moveBookmarkToPosition(draggedId, item.id, dropAbove); - }); + this.attachBookmarkDragHandlers(rowEl, container, item, false); // Context menu rowEl.addEventListener('contextmenu', (event: MouseEvent) => { @@ -558,45 +530,7 @@ export class WaypointView extends ItemView { rowEl.style.cursor = 'grab'; // Drag events - rowEl.addEventListener('dragstart', (e) => { - this.dragId = item.id; - e.dataTransfer!.effectAllowed = 'move'; - e.dataTransfer!.setData('text/plain', item.id); - rowEl.addClass('waypoint-bm-dragging'); - }); - rowEl.addEventListener('dragend', () => { - this.dragId = null; - container.querySelectorAll('.waypoint-bm-dragging, .waypoint-bm-drop-line, .waypoint-bm-drop-below, .waypoint-bm-drop-into').forEach(el => { - el.removeClass('waypoint-bm-dragging'); - el.removeClass('waypoint-bm-drop-line'); - el.removeClass('waypoint-bm-drop-below'); - }); - }); - rowEl.addEventListener('dragenter', (e) => { - e.preventDefault(); - if (!this.dragId || this.dragId === item.id) return; - this.showDropIndicator(rowEl, e, false); - }); - rowEl.addEventListener('dragover', (e) => { - e.preventDefault(); - if (!this.dragId || this.dragId === item.id) return; - this.showDropIndicator(rowEl, e, false); - }); - rowEl.addEventListener('dragleave', () => { - rowEl.removeClass('waypoint-bm-drop-line'); - rowEl.removeClass('waypoint-bm-drop-below'); - rowEl.removeClass('waypoint-bm-drop-into'); - }); - rowEl.addEventListener('drop', (e) => { - e.preventDefault(); - this.dragId = null; - rowEl.removeClass('waypoint-bm-drop-line'); - rowEl.removeClass('waypoint-bm-drop-below'); - const draggedId = e.dataTransfer?.getData('text/plain'); - if (!draggedId || draggedId === item.id) return; - const dropAbove = (rowEl as any).__dropAbove; - this.moveBookmarkToPosition(draggedId, item.id, dropAbove); - }); + this.attachBookmarkDragHandlers(rowEl, container, item, false); // Context menu rowEl.addEventListener('contextmenu', (event: MouseEvent) => { @@ -619,65 +553,7 @@ export class WaypointView extends ItemView { } // ── Drag events ── - rowEl.addEventListener('dragstart', (e) => { - this.dragId = item.id; - e.dataTransfer!.effectAllowed = 'move'; - e.dataTransfer!.setData('text/plain', item.id); - rowEl.addClass('waypoint-bm-dragging'); - }); - - const canAcceptChildren = true; // all file/group bookmarks can accept drops - - rowEl.addEventListener('dragend', () => { - this.dragId = null; - container.querySelectorAll('.waypoint-bm-dragging, .waypoint-bm-drop-line, .waypoint-bm-drop-below, .waypoint-bm-drop-into').forEach(el => { - el.removeClass('waypoint-bm-dragging'); - el.removeClass('waypoint-bm-drop-line'); - el.removeClass('waypoint-bm-drop-below'); - el.removeClass('waypoint-bm-drop-into'); - }); - }); - - rowEl.addEventListener('dragenter', (e) => { - e.preventDefault(); - if (!this.dragId || this.dragId === item.id) return; - this.showDropIndicator(rowEl, e, canAcceptChildren); - }); - - rowEl.addEventListener('dragover', (e) => { - e.preventDefault(); - if (!this.dragId || this.dragId === item.id) return; - this.showDropIndicator(rowEl, e, canAcceptChildren); - }); - - rowEl.addEventListener('dragleave', () => { - rowEl.removeClass('waypoint-bm-drop-line'); - rowEl.removeClass('waypoint-bm-drop-below'); - rowEl.removeClass('waypoint-bm-drop-into'); - }); - - rowEl.addEventListener('drop', (e) => { - e.preventDefault(); - this.dragId = null; - rowEl.removeClass('waypoint-bm-drop-line'); - rowEl.removeClass('waypoint-bm-drop-below'); - rowEl.removeClass('waypoint-bm-drop-into'); - - const draggedId = e.dataTransfer?.getData('text/plain'); - if (!draggedId || draggedId === item.id) return; - - const dropInto = (rowEl as any).__dropInto; - if (dropInto && canAcceptChildren) { - if (isGroup) { - this.moveBookmarkToGroup(draggedId, item.id); - } else { - this.createParentNoteAndMove(draggedId, item.id); - } - } else { - const dropAbove = (rowEl as any).__dropAbove; - this.moveBookmarkToPosition(draggedId, item.id, dropAbove); - } - }); + this.attachBookmarkDragHandlers(rowEl, container, item, true); // ── Group: chevron + icon + label ── if (isGroup) { @@ -699,7 +575,7 @@ export class WaypointView extends ItemView { const tfile = this.app.vault.getFileByPath(item.filePath); if (tfile) { const newLeaf = Keymap.isModEvent(event); - this.app.workspace.getLeaf(newLeaf as any).openFile(tfile); + this.app.workspace.getLeaf(newLeaf).openFile(tfile); return; } } @@ -751,7 +627,7 @@ export class WaypointView extends ItemView { const tfile = this.app.vault.getFileByPath(item.filePath); if (tfile) { const newLeaf = Keymap.isModEvent(event); - this.app.workspace.getLeaf(newLeaf as any).openFile(tfile); + this.app.workspace.getLeaf(newLeaf).openFile(tfile); } else { new Notice('File not found'); this.plugin.removeBookmark(item.id); @@ -935,6 +811,75 @@ export class WaypointView extends ItemView { menu.showAtPosition({ x: event.clientX, y: event.clientY }); } + /** + * Attach the shared bookmark drag-and-drop listeners to a row element. + * `canAcceptChildren` selects 3-zone (above/into/below) drop targeting for + * file and group rows; separators and spacers use 2-zone (above/below). + */ + private attachBookmarkDragHandlers( + rowEl: HTMLElement, + container: HTMLElement, + item: BookmarkItem, + canAcceptChildren: boolean, + ): void { + const clearIndicators = (): void => { + rowEl.removeClass('waypoint-bm-drop-line'); + rowEl.removeClass('waypoint-bm-drop-below'); + rowEl.removeClass('waypoint-bm-drop-into'); + }; + + rowEl.addEventListener('dragstart', (e) => { + this.dragId = item.id; + e.dataTransfer!.effectAllowed = 'move'; + e.dataTransfer!.setData('text/plain', item.id); + rowEl.addClass('waypoint-bm-dragging'); + }); + + rowEl.addEventListener('dragend', () => { + this.dragId = null; + container.querySelectorAll('.waypoint-bm-dragging, .waypoint-bm-drop-line, .waypoint-bm-drop-below, .waypoint-bm-drop-into').forEach(el => { + el.removeClass('waypoint-bm-dragging'); + el.removeClass('waypoint-bm-drop-line'); + el.removeClass('waypoint-bm-drop-below'); + el.removeClass('waypoint-bm-drop-into'); + }); + }); + + rowEl.addEventListener('dragenter', (e) => { + e.preventDefault(); + if (!this.dragId || this.dragId === item.id) return; + this.showDropIndicator(rowEl, e, canAcceptChildren); + }); + + rowEl.addEventListener('dragover', (e) => { + e.preventDefault(); + if (!this.dragId || this.dragId === item.id) return; + this.showDropIndicator(rowEl, e, canAcceptChildren); + }); + + rowEl.addEventListener('dragleave', clearIndicators); + + rowEl.addEventListener('drop', (e) => { + e.preventDefault(); + this.dragId = null; + clearIndicators(); + + const draggedId = e.dataTransfer?.getData('text/plain'); + if (!draggedId || draggedId === item.id) return; + + const zone = this.dropZones.get(rowEl); + if (canAcceptChildren && zone?.into) { + if (item.type === 'group') { + this.moveBookmarkToGroup(draggedId, item.id); + } else { + this.createParentNoteAndMove(draggedId, item.id); + } + } else { + this.moveBookmarkToPosition(draggedId, item.id, zone?.above ?? false); + } + }); + } + private showDropIndicator(el: HTMLElement, e: MouseEvent, isGroupLike: boolean): void { // Clear all indicators const parent = el.parentElement; @@ -956,25 +901,21 @@ export class WaypointView extends ItemView { if (y < topThreshold) { el.addClass('waypoint-bm-drop-line'); - (el as any).__dropAbove = true; - (el as any).__dropInto = false; + this.dropZones.set(el, { above: true, into: false }); } else if (y > bottomThreshold) { el.addClass('waypoint-bm-drop-line'); el.addClass('waypoint-bm-drop-below'); - (el as any).__dropAbove = false; - (el as any).__dropInto = false; + this.dropZones.set(el, { above: false, into: false }); } else { el.addClass('waypoint-bm-drop-into'); - (el as any).__dropAbove = false; - (el as any).__dropInto = true; + this.dropZones.set(el, { above: false, into: true }); } } else { // 2-zone: top half = above, bottom half = below const above = y < rect.top + rect.height / 2; el.addClass('waypoint-bm-drop-line'); if (!above) el.addClass('waypoint-bm-drop-below'); - (el as any).__dropAbove = above; - (el as any).__dropInto = false; + this.dropZones.set(el, { above, into: false }); } } @@ -1130,9 +1071,8 @@ export class WaypointView extends ItemView { private promptIcon(item: BookmarkItem): void { new IconSuggestModal(this.app, item.icon, (iconName) => { - if (iconName) { - this.plugin.updateBookmark(item.id, { icon: iconName }); - } + // An empty string is a valid value meaning "no icon", so always apply. + this.plugin.updateBookmark(item.id, { icon: iconName }); }).open(); } } @@ -1188,6 +1128,31 @@ class RenameModal extends Modal { // ── Icon picker modal (full Lucide icon set, grid layout) ── +// Lucide icon metadata, fetched at most once per session and shared by every +// picker. Concurrent opens await the same in-flight promise; a failed fetch is +// not cached, so a later open retries once the network is back. +let iconCatalog: Promise> | null = null; + +function loadIconCatalog(): Promise> { + if (!iconCatalog) { + iconCatalog = (async () => { + try { + const res = await fetch('https://cdn.jsdelivr.net/npm/lucide-static@0.517.0/tags.json'); + return (await res.json()) as Record; + } catch { + try { + const res = await fetch('https://lucide.dev/api/tags'); + return (await res.json()) as Record; + } catch { + iconCatalog = null; + return FALLBACK_ICONS; + } + } + })(); + } + return iconCatalog; +} + class IconSuggestModal extends Modal { private onSubmit: (icon: string) => void; private selected: string; @@ -1280,7 +1245,7 @@ class IconSuggestModal extends Modal { }); // ── Render grid ── - let debounce: any = null; + let debounce: number | undefined; const renderGrid = (query: string) => { grid.empty(); @@ -1306,9 +1271,8 @@ class IconSuggestModal extends Modal { return; } - for (var i = 0; i < matches.length; i++) { - var name = matches[i]; - var tile = grid.createDiv(); + for (const name of matches) { + const tile = grid.createDiv(); tile.setAttr('data-icon', name); tile.style.display = 'flex'; tile.style.alignItems = 'center'; @@ -1326,60 +1290,58 @@ class IconSuggestModal extends Modal { tile.style.color = 'var(--text-muted)'; } - var svg = tile.createSpan(); + const svg = tile.createSpan(); svg.style.display = 'flex'; setIcon(svg, name); - ;(function(_self, _tile, _name, _query, _grid, _previewIcon, _previewLabel) { - _tile.addEventListener('mouseenter', function() { - if (_name !== _self.selected) _tile.style.background = 'var(--background-modifier-hover)'; - }); - _tile.addEventListener('mouseleave', function() { - if (_name !== _self.selected) _tile.style.background = ''; - }); + tile.addEventListener('mouseenter', () => { + if (name !== this.selected) tile.style.background = 'var(--background-modifier-hover)'; + }); + tile.addEventListener('mouseleave', () => { + if (name !== this.selected) tile.style.background = ''; + }); - _tile.addEventListener('click', function() { - _self.selected = _name; - renderGrid(_query); - _previewIcon.empty(); - setIcon(_previewIcon, _name); - _previewLabel.setText(_name); - _grid.querySelectorAll('div[data-icon]').forEach(function(_el) { - var el = _el as HTMLElement; - if (el.getAttr('data-icon') === _name) { - el.style.background = 'var(--interactive-accent)'; - el.style.color = 'var(--text-on-accent)'; - } else { - el.style.background = ''; - el.style.color = 'var(--text-muted)'; - } - }); + tile.addEventListener('click', () => { + this.selected = name; + renderGrid(query); + previewIcon.empty(); + setIcon(previewIcon, name); + previewLabel.setText(name); + grid.querySelectorAll('div[data-icon]').forEach((node) => { + const el = node as HTMLElement; + if (el.getAttr('data-icon') === name) { + el.style.background = 'var(--interactive-accent)'; + el.style.color = 'var(--text-on-accent)'; + } else { + el.style.background = ''; + el.style.color = 'var(--text-muted)'; + } }); - })(this, tile, name, query, grid, previewIcon, previewLabel); + }); } statusEl.setText(matches.length + ' of ' + this.allIcons.length + ' icons'); }; - input.addEventListener('input', function() { - if (debounce !== null) clearTimeout(debounce); - debounce = setTimeout(function() { renderGrid(input.value); }, 60); + input.addEventListener('input', () => { + window.clearTimeout(debounce); + debounce = window.setTimeout(() => renderGrid(input.value), 60); }); - input.addEventListener('keydown', function(e: KeyboardEvent) { + input.addEventListener('keydown', (e: KeyboardEvent) => { if (e.key === 'Escape') this.close(); - }.bind(this)); + }); // ── Buttons ── - var btns = modal.createDiv({ cls: 'modal-button-container' }); - var cancel = btns.createEl('button', { text: 'Cancel' }); - cancel.addEventListener('click', function() { this.close(); }.bind(this)); - var saveBtn = btns.createEl('button', { text: 'Save', cls: 'mod-cta' }); + const btns = modal.createDiv({ cls: 'modal-button-container' }); + const cancel = btns.createEl('button', { text: 'Cancel' }); + cancel.addEventListener('click', () => this.close()); + const saveBtn = btns.createEl('button', { text: 'Save', cls: 'mod-cta' }); saveBtn.style.marginLeft = '8px'; - saveBtn.addEventListener('click', function() { + saveBtn.addEventListener('click', () => { this.onSubmit(this.selected); this.close(); - }.bind(this)); + }); } onClose(): void { @@ -1387,22 +1349,9 @@ class IconSuggestModal extends Modal { } private async loadIcons(): Promise { - try { - var r = await fetch('https://cdn.jsdelivr.net/npm/lucide-static@0.517.0/tags.json'); - var d = await r.json(); - this.tagsMap = d as Record; - this.allIcons = Object.keys(d).sort(); - } catch (_e) { - try { - var r2 = await fetch('https://lucide.dev/api/tags'); - var d2 = await r2.json(); - this.tagsMap = d2 as Record; - this.allIcons = Object.keys(d2).sort(); - } catch (_e2) { - this.tagsMap = {}; - this.allIcons = Object.keys(FALLBACK_ICONS).sort(); - } - } + const catalog = await loadIconCatalog(); + this.tagsMap = catalog; + this.allIcons = Object.keys(catalog).sort(); } }