From 43ebec00eb72e95c4d1e47029d7a8738a8710792 Mon Sep 17 00:00:00 2001 From: Scott Tomaszewski Date: Wed, 16 Apr 2025 10:36:19 -0400 Subject: [PATCH] Satisfies code reviews and supports popout windows --- src/components/BibleReferenceRenderer.ts | 34 ++++++------ src/components/BibleStyles.ts | 8 +-- src/core/BibleEventHandlers.ts | 12 ++--- src/core/DisciplesJournalPlugin.ts | 66 ++++++++---------------- src/services/ESVApiService.ts | 21 ++++---- src/services/NoteCreationService.ts | 2 +- src/settings/DisciplesJournalSettings.ts | 2 +- 7 files changed, 62 insertions(+), 83 deletions(-) diff --git a/src/components/BibleReferenceRenderer.ts b/src/components/BibleReferenceRenderer.ts index 0e8fcaa..bf071dc 100644 --- a/src/components/BibleReferenceRenderer.ts +++ b/src/components/BibleReferenceRenderer.ts @@ -128,7 +128,7 @@ export class BibleReferenceRenderer { } // Create a Bible reference element - const referenceEl = document.createElement('span'); + const referenceEl = element.doc.createElement('span'); referenceEl.classList.add('bible-reference'); referenceEl.textContent = codeText; @@ -149,7 +149,7 @@ export class BibleReferenceRenderer { const passage = await this.bibleContentService.getBibleContent(reference); if (passage) { - const containerEl = document.createElement('div'); + const containerEl = el.doc.createElement('div'); containerEl.classList.add('bible-passage-container'); const parsedRef = this.parser.parse(reference); @@ -186,7 +186,7 @@ export class BibleReferenceRenderer { // } // Add reference heading - const headingEl = document.createElement('h3'); + const headingEl = el.doc.createElement('h3'); headingEl.classList.add('bible-passage-heading'); const referenceLink = headingEl.createEl('a', { text: passage.reference }); referenceLink.onClickEvent(async l => { @@ -196,7 +196,7 @@ export class BibleReferenceRenderer { containerEl.appendChild(headingEl); // Add verses - const passageEl = document.createElement('div'); + const passageEl = el.doc.createElement('div'); passageEl.classList.add('bible-passage-text'); // Check if we have HTML content @@ -206,14 +206,14 @@ export class BibleReferenceRenderer { } else { // Fallback to traditional verse rendering for (const verse of passage.verses) { - const verseEl = document.createElement('p'); + const verseEl = el.doc.createElement('p'); verseEl.classList.add('bible-verse'); - const verseNumEl = document.createElement('span'); + const verseNumEl = el.doc.createElement('span'); verseNumEl.classList.add('bible-verse-number'); verseNumEl.textContent = `${verse.verse} `; - const verseTextEl = document.createElement('span'); + const verseTextEl = el.doc.createElement('span'); verseTextEl.classList.add('bible-verse-text'); verseTextEl.textContent = verse.text; @@ -228,7 +228,7 @@ export class BibleReferenceRenderer { el.appendChild(containerEl); } else { // If reference not found, show error - const errorEl = document.createElement('div'); + const errorEl = el.doc.createElement('div'); errorEl.classList.add('bible-reference-error'); errorEl.textContent = `Bible reference "${reference}" not found.`; el.appendChild(errorEl); @@ -243,11 +243,11 @@ export class BibleReferenceRenderer { if (!passage) return null; // Create verse preview element - const versePreviewEl = document.createElement('div'); + const versePreviewEl = element.doc.createElement('div'); versePreviewEl.classList.add('bible-verse-preview'); // Add reference heading (make it clickable) - const headingEl = document.createElement('div'); + const headingEl = element.doc.createElement('div'); headingEl.classList.add('bible-verse-preview-heading', 'bible-reference-clickable'); headingEl.textContent = passage.reference; @@ -263,7 +263,7 @@ export class BibleReferenceRenderer { // Close the preview - using a method available in BibleEventHandlers // Remove the popup directly instead of trying to access the private property - const previewPoppers = document.querySelectorAll('.bible-verse-preview'); + const previewPoppers = element.doc.querySelectorAll('.bible-verse-preview'); if (previewPoppers) { previewPoppers.forEach(p => p.remove()); } @@ -278,7 +278,7 @@ export class BibleReferenceRenderer { versePreviewEl.appendChild(headingEl); // Add verse content - const contentEl = document.createElement('div'); + const contentEl = element.doc.createElement('div'); contentEl.classList.add('bible-verse-preview-content'); // Check if we have HTML content @@ -287,7 +287,7 @@ export class BibleReferenceRenderer { // for the preview (to avoid showing footnotes, chapter headings, etc.) try { // Create a temporary element to parse the HTML - const tempEl = document.createElement('div'); + const tempEl = element.doc.createElement('div'); tempEl.innerHTML = passage.htmlContent; // Find and extract the main verse content (paragraphs) @@ -307,16 +307,16 @@ export class BibleReferenceRenderer { } else { // Fallback to traditional verse rendering for (const verse of passage.verses) { - const verseEl = document.createElement('p'); + const verseEl = element.doc.createElement('p'); if (passage.verses.length > 1) { - const verseNumEl = document.createElement('span'); + const verseNumEl = element.doc.createElement('span'); verseNumEl.classList.add('bible-verse-number'); verseNumEl.textContent = `${verse.verse} `; verseEl.appendChild(verseNumEl); } - const verseTextEl = document.createElement('span'); + const verseTextEl = element.doc.createElement('span'); verseTextEl.textContent = verse.text; verseEl.appendChild(verseTextEl); @@ -335,7 +335,7 @@ export class BibleReferenceRenderer { versePreviewEl.style.top = `${rect.bottom - 3}px`; // Add popup to document - document.body.appendChild(versePreviewEl); + element.doc.body.appendChild(versePreviewEl); return versePreviewEl; } diff --git a/src/components/BibleStyles.ts b/src/components/BibleStyles.ts index e2eb7c1..6050e7b 100644 --- a/src/components/BibleStyles.ts +++ b/src/components/BibleStyles.ts @@ -86,7 +86,7 @@ export class BibleStyles { /** * Apply styles by updating CSS variables based on theme, preset, and settings. */ - public applyStyles(theme: 'light' | 'dark', presetName: string = 'default', fontSize: string = '100%', settings?: { + public applyStyles(doc: Document, theme: 'light' | 'dark', presetName: string = 'default', fontSize: string = '100%', settings?: { wordsOfChristColor?: string; verseNumberColor?: string; headingColor?: string; @@ -115,7 +115,7 @@ export class BibleStyles { } // Update the CSS variables on the root element - const rootStyle = document.documentElement.style; + const rootStyle = doc.body.style; rootStyle.setProperty('--dj-font-size', options.fontSize); rootStyle.setProperty('--dj-heading-color', options.headingColor); rootStyle.setProperty('--dj-verse-number-color', options.verseNumberColor); @@ -127,8 +127,8 @@ export class BibleStyles { * Resets the custom styles potentially set by this class by removing the CSS variables. * This allows the default styles in styles.css to take effect. */ - public resetStyles(): void { - const rootStyle = document.documentElement.style; + public resetStyles(doc: Document): void { + const rootStyle = doc.body.style; rootStyle.removeProperty('--dj-font-size'); rootStyle.removeProperty('--dj-heading-color'); rootStyle.removeProperty('--dj-verse-number-color'); diff --git a/src/core/BibleEventHandlers.ts b/src/core/BibleEventHandlers.ts index 3d9c759..6697b11 100644 --- a/src/core/BibleEventHandlers.ts +++ b/src/core/BibleEventHandlers.ts @@ -100,7 +100,7 @@ export class BibleEventHandlers { !relatedTarget.classList.contains('bible-reference') && !relatedTarget.closest('.bible-verse-preview')) { this.previewPopper?.classList.remove('popup-locked'); - this.removePreviewPopper(); + this.removePreviewPopper(relatedTarget.doc); } }); } @@ -125,7 +125,7 @@ export class BibleEventHandlers { if (!this.previewPopper || this.previewPopper.classList.contains('popup-locked')) { return; } - this.removePreviewPopper(); + this.removePreviewPopper(relatedTarget.doc); }, 100); } }); @@ -163,20 +163,20 @@ export class BibleEventHandlers { } // In all other cases, remove the popup - this.removePreviewPopper(); + this.removePreviewPopper(target.doc); } /** * Remove the preview popper if it exists */ - removePreviewPopper() { + removePreviewPopper(doc: Document) { if (this.previewPopper) { // Remove any hover gap elements - const hoverGaps = document.querySelectorAll('.bible-hover-gap'); + const hoverGaps = doc.querySelectorAll('.bible-hover-gap'); hoverGaps.forEach(gap => gap.remove()); this.previewPopper.remove(); this.previewPopper = null; } } -} \ No newline at end of file +} \ No newline at end of file diff --git a/src/core/DisciplesJournalPlugin.ts b/src/core/DisciplesJournalPlugin.ts index e4e7c45..6eba203 100644 --- a/src/core/DisciplesJournalPlugin.ts +++ b/src/core/DisciplesJournalPlugin.ts @@ -8,8 +8,8 @@ import { BibleReferenceRenderer } from '../components/BibleReferenceRenderer'; import { BibleStyles } from '../components/BibleStyles'; import { DisciplesJournalSettings, DEFAULT_SETTINGS, DisciplesJournalSettingsTab } from '../settings/DisciplesJournalSettings'; import { BibleFormatter } from "../utils/BibleFormatter"; +import { NoteCreationService } from 'src/services/NoteCreationService'; import { BibleEventHandlers } from './BibleEventHandlers'; -import { NoteCreationService } from '../services/NoteCreationService'; import { BibleMarkupProcessor } from './BibleMarkupProcessor'; /** @@ -62,7 +62,7 @@ export default class DisciplesJournalPlugin extends Plugin { // Initialize components this.bibleReferenceParser = new BibleReferenceParser(this.bookNameService); - this.bibleStyles = new BibleStyles(this.settings.bibleTextFontSize); + this.bibleStyles = new BibleStyles(); this.bibleReferenceRenderer = new BibleReferenceRenderer( this.app, this.bibleContentService, @@ -118,8 +118,6 @@ export default class DisciplesJournalPlugin extends Plugin { } onunload() { - console.log('Unloading Disciples Journal plugin'); - this.bibleStyles.removeStyles(); } async loadSettings() { @@ -167,47 +165,25 @@ export default class DisciplesJournalPlugin extends Plugin { const activeLeaf = this.app.workspace.getActiveViewOfType(MarkdownView); if (!activeLeaf) return; - const isDarkMode = document.body.classList.contains('theme-dark'); - const theme = isDarkMode ? 'dark' : 'light'; - - // Pass custom settings from DisciplesJournalSettings - this.bibleStyles.applyStyles( - theme, - this.settings.stylePreset, - this.settings.bibleTextFontSize, - { - wordsOfChristColor: this.settings.wordsOfChristColor, - verseNumberColor: this.settings.verseNumberColor, - headingColor: this.settings.headingColor, - blockIndentation: this.settings.blockIndentation - } - ); - } - - /** - * Update font size for Bible verses (legacy method for compatibility) - */ - public updateFontSize(fontSize: string): void { - this.bibleStyles.setFontSize(fontSize); - - // Refresh the styles with the new font size and current settings - const isDarkMode = document.body.classList.contains('theme-dark'); - const theme = isDarkMode ? 'dark' : 'light'; - - // Apply styles with the new font size and current settings - this.bibleStyles.applyStyles( - theme, - this.settings.stylePreset, - fontSize, - { - wordsOfChristColor: this.settings.wordsOfChristColor, - verseNumberColor: this.settings.verseNumberColor, - headingColor: this.settings.headingColor, - blockIndentation: this.settings.blockIndentation - } - ); - - this.bibleReferenceRenderer.setFontSize(fontSize); + // Each popup window has a unique document, so we need to apply styles to each one + const docList: Document[] = []; + this.app.workspace.iterateAllLeaves(leaf => docList.push(leaf.getContainer().doc)); + docList.unique().forEach(doc => { + const isDarkMode = activeLeaf.containerEl.doc.body.classList.contains('theme-dark'); + const theme = isDarkMode ? 'dark' : 'light'; + this.bibleStyles.applyStyles( + doc, + theme, + this.settings.stylePreset, + this.settings.bibleTextFontSize, + { + wordsOfChristColor: this.settings.wordsOfChristColor, + verseNumberColor: this.settings.verseNumberColor, + headingColor: this.settings.headingColor, + blockIndentation: this.settings.blockIndentation + } + ); + }); } /** diff --git a/src/services/ESVApiService.ts b/src/services/ESVApiService.ts index c9d3478..a712738 100644 --- a/src/services/ESVApiService.ts +++ b/src/services/ESVApiService.ts @@ -161,42 +161,42 @@ export class ESVApiService { /** * Create a DOM-friendly error message for missing token or API errors */ - private createErrorMessageContent(type: 'missing-token' | 'api-error', message: string = '', details: string = ''): HTMLElement { + private createErrorMessageContent(doc: Document, type: 'missing-token' | 'api-error', message: string = '', details: string = ''): HTMLElement { // We're required to return a string here because it's part of the ESV API response structure // This method creates the content in a DOM-safe way then serializes to a string - const container = document.createElement('div'); + const container = doc.createElement('div'); container.className = type === 'missing-token' ? 'bible-missing-token-warning' : 'bible-api-error'; - const titleEl = document.createElement('p'); - const titleStrong = document.createElement('strong'); + const titleEl = doc.createElement('p'); + const titleStrong = doc.createElement('strong'); titleStrong.textContent = type === 'missing-token' ? 'ESV API Token Not Set' : 'Error Loading Bible Passage'; titleEl.appendChild(titleStrong); container.appendChild(titleEl); if (message) { - const messageEl = document.createElement('p'); + const messageEl = doc.createElement('p'); messageEl.textContent = message; container.appendChild(messageEl); } if (details) { - const detailsEl = document.createElement('p'); + const detailsEl = doc.createElement('p'); detailsEl.textContent = details; container.appendChild(detailsEl); } // For the API token link if (type === 'missing-token') { - const linkPara = document.createElement('p'); + const linkPara = doc.createElement('p'); linkPara.textContent = 'You can request a free token from '; - const link = document.createElement('a'); + const link = doc.createElement('a'); link.textContent = 'api.esv.org'; link.href = 'https://api.esv.org/docs/'; link.target = '_blank'; linkPara.appendChild(link); - linkPara.appendChild(document.createTextNode('.')); + linkPara.appendChild(doc.createTextNode('.')); container.appendChild(linkPara); } @@ -214,6 +214,7 @@ export class ESVApiService { reference: reference, verses: [], htmlContent: this.createErrorMessageContent( + document, 'missing-token', 'To display Bible passages, you need to set up an ESV API token in the plugin settings.' ).outerHTML, @@ -257,6 +258,7 @@ export class ESVApiService { reference: reference, verses: [], htmlContent: this.createErrorMessageContent( + document, 'api-error', 'Failed to load the passage from the ESV API.', `Status: ${response.status}. Please check your API token in the plugin settings.` @@ -270,6 +272,7 @@ export class ESVApiService { reference: reference, verses: [], htmlContent: this.createErrorMessageContent( + document, 'api-error', 'An error occurred when trying to access the ESV API.', 'Please check your internet connection and API token.' diff --git a/src/services/NoteCreationService.ts b/src/services/NoteCreationService.ts index 04bd05f..b2f18ed 100644 --- a/src/services/NoteCreationService.ts +++ b/src/services/NoteCreationService.ts @@ -68,7 +68,7 @@ export class NoteCreationService { if (parsedRef.verse) { setTimeout(() => { // Find the verse element and scroll to it - const verseEl = document.querySelector(`.verse-${parsedRef.verse}`); + const verseEl = leaf.getContainer().doc.querySelector(`.verse-${parsedRef.verse}`); if (verseEl) { verseEl.scrollIntoView({ behavior: 'smooth', block: 'center' }); } diff --git a/src/settings/DisciplesJournalSettings.ts b/src/settings/DisciplesJournalSettings.ts index 8658539..f8ec19d 100644 --- a/src/settings/DisciplesJournalSettings.ts +++ b/src/settings/DisciplesJournalSettings.ts @@ -110,8 +110,8 @@ export class DisciplesJournalSettingsTab extends PluginSettingTab { .setValue(this.plugin.settings.bibleTextFontSize) .onChange(async (value) => { this.plugin.settings.bibleTextFontSize = value; - this.plugin.updateFontSize(value); await this.plugin.saveSettings(); + this.plugin.updateBibleStyles(); }) );