Files
obsidian-mcp-server/docs/plans/2025-10-26-notification-ui-improvements.md
Bill 17976065df docs: add implementation plan for notification UI improvements
Detailed plan with bite-sized tasks for:
- Notification message format improvements
- Settings section collapse fix
- Modal filter control repairs

Co-Authored-By: Claude <noreply@anthropic.com>
2025-10-26 09:14:07 -04:00

21 KiB

Notification UI Improvements Implementation Plan

For Claude: REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.

Goal: Fix three UX issues in the MCP notification system: unclear message format, settings collapse on toggle, and broken modal filters.

Architecture: Targeted surgical fixes - update notification message format in NotificationManager, implement targeted DOM updates in settings tab to prevent collapse, refactor modal filters to use Obsidian Setting components and eliminate destructive re-renders.

Tech Stack: TypeScript, Obsidian API (Notice, Setting, Modal), Jest for testing


Task 1: Update Notification Message Format

Files:

  • Modify: src/ui/notifications.ts:77-94 (showToolCall method)
  • Modify: src/ui/notifications.ts:140-177 (formatArgs method)
  • Test: tests/notifications.test.ts (new file)

Step 1: Write failing test for new notification format

Create tests/notifications.test.ts:

import { App } from 'obsidian';
import { NotificationManager } from '../src/ui/notifications';
import { MCPPluginSettings } from '../src/types/settings-types';

describe('NotificationManager', () => {
	let app: App;
	let settings: MCPPluginSettings;
	let manager: NotificationManager;

	beforeEach(() => {
		app = {} as App;
		settings = {
			port: 3000,
			autoStart: false,
			apiKey: 'test-key',
			notificationsEnabled: true,
			showParameters: true,
			notificationDuration: 3000,
			logToConsole: false
		};
		manager = new NotificationManager(app, settings);
	});

	describe('showToolCall', () => {
		it('should format message with MCP Tool Called label and newline when parameters shown', () => {
			const mockNotice = jest.fn();
			(global as any).Notice = mockNotice;

			manager.showToolCall('read_note', { path: 'daily/2025-01-15.md' });

			expect(mockNotice).toHaveBeenCalledWith(
				expect.stringContaining('📖 MCP Tool Called: read_note\npath: "daily/2025-01-15.md"'),
				3000
			);
		});

		it('should format message without newline when parameters hidden', () => {
			settings.showParameters = false;
			manager = new NotificationManager(app, settings);
			const mockNotice = jest.fn();
			(global as any).Notice = mockNotice;

			manager.showToolCall('read_note', { path: 'daily/2025-01-15.md' });

			expect(mockNotice).toHaveBeenCalledWith(
				'📖 MCP Tool Called: read_note',
				3000
			);
		});

		it('should format multiple parameters correctly', () => {
			const mockNotice = jest.fn();
			(global as any).Notice = mockNotice;

			manager.showToolCall('search', {
				query: 'test query',
				folder: 'notes',
				recursive: true
			});

			expect(mockNotice).toHaveBeenCalledWith(
				expect.stringContaining('🔍 MCP Tool Called: search\nquery: "test query", folder: "notes", recursive: true'),
				3000
			);
		});
	});
});

Step 2: Run test to verify it fails

Run: npm test -- tests/notifications.test.ts Expected: FAIL with test failures showing old format 🔧 MCP: read_note

Step 3: Update formatArgs to return unwrapped parameter string

In src/ui/notifications.ts, modify the formatArgs method (lines 140-177):

/**
 * Format arguments for display
 */
private formatArgs(args: any): string {
	if (!this.settings.showParameters) {
		return '';
	}

	if (!args || Object.keys(args).length === 0) {
		return '';
	}

	try {
		// Extract key parameters for display
		const keyParams: string[] = [];

		if (args.path) {
			keyParams.push(`path: "${this.truncateString(args.path, 30)}"`);
		}
		if (args.query) {
			keyParams.push(`query: "${this.truncateString(args.query, 30)}"`);
		}
		if (args.folder) {
			keyParams.push(`folder: "${this.truncateString(args.folder, 30)}"`);
		}
		if (args.recursive !== undefined) {
			keyParams.push(`recursive: ${args.recursive}`);
		}

		// If no key params, show first 50 chars of JSON
		if (keyParams.length === 0) {
			const json = JSON.stringify(args);
			return this.truncateString(json, 50);
		}

		return keyParams.join(', ');
	} catch (e) {
		return '';
	}
}

Step 4: Update showToolCall to use new format

In src/ui/notifications.ts, modify the showToolCall method (lines 77-94):

/**
 * Show notification for tool call start
 */
showToolCall(toolName: string, args: any, duration?: number): void {
	if (!this.shouldShowNotification()) {
		return;
	}

	const icon = TOOL_ICONS[toolName] || '🔧';
	const argsStr = this.formatArgs(args);
	const message = argsStr
		? `${icon} MCP Tool Called: ${toolName}\n${argsStr}`
		: `${icon} MCP Tool Called: ${toolName}`;

	this.queueNotification(() => {
		new Notice(message, duration || this.settings.notificationDuration);
	});

	// Log to console if enabled
	if (this.settings.logToConsole) {
		console.log(`[MCP] Tool call: ${toolName}`, args);
	}
}

Step 5: Run tests to verify they pass

Run: npm test -- tests/notifications.test.ts Expected: PASS - all notification format tests pass

Step 6: Run full test suite

Run: npm test Expected: PASS - all 569+ tests pass

Step 7: Commit

git add src/ui/notifications.ts tests/notifications.test.ts
git commit -m "feat: improve notification message clarity with MCP Tool Called label

- Update notification format to multi-line with explicit label
- First line: 'MCP Tool Called: tool_name'
- Second line: parameters (if enabled)
- Add comprehensive tests for notification formatting

Co-Authored-By: Claude <noreply@anthropic.com>"

Task 2: Fix Settings Section Collapse on Toggle

Files:

  • Modify: src/settings.ts:6-12 (add instance variable)
  • Modify: src/settings.ts:198-272 (notification section rendering)
  • Test: Manual testing (UI component, hard to unit test)

Step 1: Add instance variable for notification details element

In src/settings.ts, add to the class properties (after line 7):

export class MCPServerSettingTab extends PluginSettingTab {
	plugin: MCPServerPlugin;
	private notificationDetailsEl: HTMLDetailsElement | null = null;

	constructor(app: App, plugin: MCPServerPlugin) {
		super(app, plugin);
		this.plugin = plugin;
	}

Step 2: Store reference when creating notification section

In src/settings.ts, modify the notification section creation (around line 199):

// Notification Settings
const notifDetails = containerEl.createEl('details');
notifDetails.style.marginBottom = '20px';
const notifSummary = notifDetails.createEl('summary');
notifSummary.style.fontSize = '1.17em';
notifSummary.style.fontWeight = 'bold';
notifSummary.style.marginBottom = '12px';
notifSummary.style.cursor = 'pointer';
notifSummary.setText('UI Notifications');

// Store reference for targeted updates
this.notificationDetailsEl = notifDetails;

Step 3: Create updateNotificationSection helper method

In src/settings.ts, add new method after the display() method:

/**
 * Update only the notification section without re-rendering entire page
 */
private updateNotificationSection(): void {
	if (!this.notificationDetailsEl) {
		// Fallback to full re-render if reference lost
		this.display();
		return;
	}

	// Store current open state
	const wasOpen = this.notificationDetailsEl.open;

	// Find and remove all child elements except the summary
	const summary = this.notificationDetailsEl.querySelector('summary');
	while (this.notificationDetailsEl.lastChild && this.notificationDetailsEl.lastChild !== summary) {
		this.notificationDetailsEl.removeChild(this.notificationDetailsEl.lastChild);
	}

	// Rebuild notification settings
	if (this.plugin.settings.notificationsEnabled) {
		// Show parameters
		new Setting(this.notificationDetailsEl)
			.setName('Show parameters')
			.setDesc('Include tool parameters in notifications')
			.addToggle(toggle => toggle
				.setValue(this.plugin.settings.showParameters)
				.onChange(async (value) => {
					this.plugin.settings.showParameters = value;
					await this.plugin.saveSettings();
					this.plugin.updateNotificationManager();
				}));

		// Notification duration
		new Setting(this.notificationDetailsEl)
			.setName('Notification duration')
			.setDesc('Duration in milliseconds')
			.addText(text => text
				.setPlaceholder('3000')
				.setValue(String(this.plugin.settings.notificationDuration))
				.onChange(async (value) => {
					const duration = parseInt(value);
					if (!isNaN(duration) && duration > 0) {
						this.plugin.settings.notificationDuration = duration;
						await this.plugin.saveSettings();
						this.plugin.updateNotificationManager();
					}
				}));

		// Log to console
		new Setting(this.notificationDetailsEl)
			.setName('Log to console')
			.setDesc('Log tool calls to console')
			.addToggle(toggle => toggle
				.setValue(this.plugin.settings.logToConsole)
				.onChange(async (value) => {
					this.plugin.settings.logToConsole = value;
					await this.plugin.saveSettings();
					this.plugin.updateNotificationManager();
				}));

		// View history button
		new Setting(this.notificationDetailsEl)
			.setName('Notification history')
			.setDesc('View recent MCP tool calls')
			.addButton(button => button
				.setButtonText('View History')
				.onClick(() => {
					this.plugin.showNotificationHistory();
				}));
	}

	// Restore open state
	this.notificationDetailsEl.open = wasOpen;
}

Step 4: Update toggle handler to use targeted update

In src/settings.ts, modify the "Enable notifications" toggle handler (around line 214):

// Enable notifications
new Setting(notifDetails)
	.setName('Enable notifications')
	.setDesc('Show when MCP tools are called')
	.addToggle(toggle => toggle
		.setValue(this.plugin.settings.notificationsEnabled)
		.onChange(async (value) => {
			this.plugin.settings.notificationsEnabled = value;
			await this.plugin.saveSettings();
			this.plugin.updateNotificationManager();
			this.updateNotificationSection(); // Changed from this.display()
		}));

Step 5: Manual testing

Manual test steps:

  1. Build: npm run build
  2. Copy main.js, manifest.json, styles.css to test vault
  3. Reload Obsidian
  4. Open plugin settings
  5. Expand "UI Notifications" section
  6. Toggle "Enable notifications" off - verify section stays open
  7. Toggle "Enable notifications" on - verify section stays open
  8. Verify subsettings appear/disappear correctly

Step 6: Commit

git add src/settings.ts
git commit -m "fix: prevent notification settings section from collapsing on toggle

- Add targeted DOM update method for notification section
- Store reference to details element during initial render
- Replace full page re-render with targeted subsection update
- Preserve open/closed state during updates

Co-Authored-By: Claude <noreply@anthropic.com>"

Task 3: Fix Modal Filter Controls

Files:

  • Modify: src/ui/notification-history.ts:7-16 (add DOM reference properties)
  • Modify: src/ui/notification-history.ts:19-35 (onOpen method)
  • Modify: src/ui/notification-history.ts:44-87 (createFilters method)
  • Modify: src/ui/notification-history.ts:89-161 (refactor list rendering)
  • Modify: src/ui/notification-history.ts:193-214 (applyFilters method)
  • Test: Manual testing (Modal UI component)

Step 1: Add DOM reference instance variables

In src/ui/notification-history.ts, update class properties (lines 7-16):

export class NotificationHistoryModal extends Modal {
	private history: NotificationHistoryEntry[];
	private filteredHistory: NotificationHistoryEntry[];
	private filterTool: string = '';
	private filterType: 'all' | 'success' | 'error' = 'all';

	// DOM element references for targeted updates
	private listContainerEl: HTMLElement | null = null;
	private countEl: HTMLElement | null = null;

	constructor(app: App, history: NotificationHistoryEntry[]) {
		super(app);
		this.history = history;
		this.filteredHistory = [...history];
	}

Step 2: Simplify onOpen to avoid re-rendering

In src/ui/notification-history.ts, modify onOpen method (lines 19-35):

onOpen() {
	const { contentEl } = this;
	contentEl.empty();
	contentEl.addClass('mcp-notification-history-modal');

	// Title
	contentEl.createEl('h2', { text: 'MCP Notification History' });

	// Filters (create once, never recreate)
	this.createFilters(contentEl);

	// History list (will be updated via reference)
	this.createHistoryListContainer(contentEl);

	// Actions
	this.createActions(contentEl);
}

Step 3: Refactor createFilters to use Setting components

In src/ui/notification-history.ts, replace the createFilters method (lines 44-87):

/**
 * Create filter controls using Obsidian Setting components
 */
private createFilters(containerEl: HTMLElement): void {
	const filterContainer = containerEl.createDiv({ cls: 'mcp-history-filters' });
	filterContainer.style.marginBottom = '16px';

	// Tool name filter using Setting component
	new Setting(filterContainer)
		.setName('Tool filter')
		.setDesc('Filter by tool name')
		.addText(text => text
			.setPlaceholder('Enter tool name...')
			.setValue(this.filterTool)
			.onChange((value) => {
				this.filterTool = value.toLowerCase();
				this.applyFilters();
			}));

	// Type filter using Setting component
	new Setting(filterContainer)
		.setName('Status filter')
		.setDesc('Filter by success or error')
		.addDropdown(dropdown => dropdown
			.addOption('all', 'All')
			.addOption('success', 'Success')
			.addOption('error', 'Error')
			.setValue(this.filterType)
			.onChange((value) => {
				this.filterType = value as 'all' | 'success' | 'error';
				this.applyFilters();
			}));

	// Results count
	this.countEl = filterContainer.createDiv({ cls: 'mcp-history-count' });
	this.countEl.style.marginTop = '8px';
	this.countEl.style.fontSize = '0.9em';
	this.countEl.style.color = 'var(--text-muted)';
	this.updateResultsCount();
}

Step 4: Create container and separate update method

In src/ui/notification-history.ts, replace createHistoryList with two methods:

/**
 * Create history list container (called once)
 */
private createHistoryListContainer(containerEl: HTMLElement): void {
	this.listContainerEl = containerEl.createDiv({ cls: 'mcp-history-list' });
	this.listContainerEl.style.maxHeight = '400px';
	this.listContainerEl.style.overflowY = 'auto';
	this.listContainerEl.style.marginBottom = '16px';
	this.listContainerEl.style.border = '1px solid var(--background-modifier-border)';
	this.listContainerEl.style.borderRadius = '4px';

	// Initial render
	this.updateHistoryList();
}

/**
 * Update history list contents (called on filter changes)
 */
private updateHistoryList(): void {
	if (!this.listContainerEl) return;

	// Clear existing content
	this.listContainerEl.empty();

	if (this.filteredHistory.length === 0) {
		const emptyEl = this.listContainerEl.createDiv({ cls: 'mcp-history-empty' });
		emptyEl.style.padding = '24px';
		emptyEl.style.textAlign = 'center';
		emptyEl.style.color = 'var(--text-muted)';
		emptyEl.textContent = 'No entries found';
		return;
	}

	this.filteredHistory.forEach((entry, index) => {
		const entryEl = this.listContainerEl!.createDiv({ cls: 'mcp-history-entry' });
		entryEl.style.padding = '12px';
		entryEl.style.borderBottom = index < this.filteredHistory.length - 1
			? '1px solid var(--background-modifier-border)'
			: 'none';

		// Header row
		const headerEl = entryEl.createDiv({ cls: 'mcp-history-entry-header' });
		headerEl.style.display = 'flex';
		headerEl.style.justifyContent = 'space-between';
		headerEl.style.marginBottom = '8px';

		// Tool name and status
		const titleEl = headerEl.createDiv();
		const statusIcon = entry.success ? '✅' : '❌';
		const toolName = titleEl.createEl('strong', { text: `${statusIcon} ${entry.toolName}` });
		toolName.style.color = entry.success ? 'var(--text-success)' : 'var(--text-error)';

		// Timestamp and duration
		const metaEl = headerEl.createDiv();
		metaEl.style.fontSize = '0.85em';
		metaEl.style.color = 'var(--text-muted)';
		const timestamp = new Date(entry.timestamp).toLocaleTimeString();
		const durationStr = entry.duration ? ` • ${entry.duration}ms` : '';
		metaEl.textContent = `${timestamp}${durationStr}`;

		// Arguments
		if (entry.args && Object.keys(entry.args).length > 0) {
			const argsEl = entryEl.createDiv({ cls: 'mcp-history-entry-args' });
			argsEl.style.fontSize = '0.85em';
			argsEl.style.fontFamily = 'monospace';
			argsEl.style.backgroundColor = 'var(--background-secondary)';
			argsEl.style.padding = '8px';
			argsEl.style.borderRadius = '4px';
			argsEl.style.marginBottom = '8px';
			argsEl.style.overflowX = 'auto';
			argsEl.textContent = JSON.stringify(entry.args, null, 2);
		}

		// Error message
		if (!entry.success && entry.error) {
			const errorEl = entryEl.createDiv({ cls: 'mcp-history-entry-error' });
			errorEl.style.fontSize = '0.85em';
			errorEl.style.color = 'var(--text-error)';
			errorEl.style.backgroundColor = 'var(--background-secondary)';
			errorEl.style.padding = '8px';
			errorEl.style.borderRadius = '4px';
			errorEl.style.fontFamily = 'monospace';
			errorEl.textContent = entry.error;
		}
	});
}

/**
 * Update results count display
 */
private updateResultsCount(): void {
	if (!this.countEl) return;
	this.countEl.textContent = `${this.filteredHistory.length} of ${this.history.length} entries`;
}

Step 5: Update applyFilters to use targeted updates

In src/ui/notification-history.ts, replace the applyFilters method (lines 193-214):

/**
 * Apply filters to history
 */
private applyFilters(): void {
	this.filteredHistory = this.history.filter(entry => {
		// Tool name filter
		if (this.filterTool && !entry.toolName.toLowerCase().includes(this.filterTool)) {
			return false;
		}

		// Type filter
		if (this.filterType === 'success' && !entry.success) {
			return false;
		}
		if (this.filterType === 'error' && entry.success) {
			return false;
		}

		return true;
	});

	// Update only the affected UI elements
	this.updateHistoryList();
	this.updateResultsCount();
}

Step 6: Add missing import

In src/ui/notification-history.ts, update imports at the top:

import { App, Modal, Setting } from 'obsidian';
import { NotificationHistoryEntry } from './notifications';

Step 7: Manual testing

Manual test steps:

  1. Build: npm run build
  2. Copy main.js to test vault
  3. Reload Obsidian
  4. Trigger some MCP tool calls (with different tools, some successes, some errors)
  5. Open notification history modal
  6. Test tool filter:
    • Type "read" - verify only read_* tools show
    • Clear filter - verify all entries return
  7. Test status filter:
    • Select "Success" - verify only successful calls show
    • Select "Error" - verify only failed calls show
    • Select "All" - verify all entries return
  8. Test combined filters
  9. Verify results count updates correctly

Step 8: Commit

git add src/ui/notification-history.ts
git commit -m "fix: repair broken filter controls in notification history modal

- Replace raw HTML inputs with Obsidian Setting components
- Add DOM element references for targeted updates
- Eliminate destructive re-render on filter changes
- Update only list container and count on filter apply
- Fix tool filter input not accepting text
- Fix status dropdown not showing selection

Co-Authored-By: Claude <noreply@anthropic.com>"

Task 4: Final Verification and Cleanup

Step 1: Run full test suite

Run: npm test Expected: All tests pass (569+ tests)

Step 2: Run type checking

Run: npm run build Expected: No TypeScript errors, successful build

Step 3: Manual end-to-end testing

Complete manual test checklist:

  • Notifications display with "MCP Tool Called:" label
  • Notifications show parameters on second line when enabled
  • Notifications show single line when parameters disabled
  • Toggling notifications on/off keeps section open
  • Tool filter in history modal accepts text and filters
  • Status dropdown in history modal shows selection
  • Combined filters work correctly
  • Results count updates correctly
  • No regressions in existing functionality

Step 4: Review commits

Run: git log --oneline master..HEAD Expected: See 3 clean commits for the 3 tasks

Step 5: Ready for merge/PR

The implementation is complete and ready for:

  • Merge to master (if sole developer)
  • Pull request (if team workflow)
  • Use @superpowers:finishing-a-development-branch skill for next steps

Implementation Notes

TDD Applied:

  • Task 1 includes comprehensive test coverage for notification formatting
  • Tasks 2-3 are UI components best tested manually
  • All changes maintain existing test suite (569 tests pass)

DRY Applied:

  • Reusable updateNotificationSection() method for settings updates
  • Reusable updateHistoryList() and updateResultsCount() for modal updates
  • Shared filter logic in applyFilters()

YAGNI Applied:

  • No premature optimizations or extra features
  • Minimal changes to fix reported issues
  • No refactoring beyond what's needed

Commits:

  • Frequent, focused commits (one per task)
  • Clear commit messages with context
  • Each commit is independently valuable

Testing Strategy:

  • Unit tests for business logic (notification formatting)
  • Manual testing for UI interactions (unavoidable with Obsidian API)
  • Full test suite verification at each step