changes in response to copilot review findings

Author: thehabes

components/classroom/auth-button.js

@@ -1,6 +1,15 @@
+import { CleanupRegistry } from '../../utilities/CleanupRegistry.js'
+
 const CENTRAL = "https://three.t-pen.org"
 
+/**
+ * AuthButton - Authentication button for login/logout functionality.
+ * @element auth-button
+ */
 class AuthButton extends HTMLElement {
+    /** @type {CleanupRegistry} Registry for cleanup handlers */
+    cleanup = new CleanupRegistry()
+
     constructor() {
         super();
         this.attachShadow({ mode: 'open' });
@@ -9,6 +18,11 @@ class AuthButton extends HTMLElement {
     connectedCallback() {
         // When the component is added to the DOM, call the render function
         this.render();
+        this.addEventListeners();
+    }
+
+    disconnectedCallback() {
+        this.cleanup.run();
     }
 
     render() {
@@ -32,10 +46,13 @@ class AuthButton extends HTMLElement {
             </style>
             <button id="auth-btn">${token ? "Logout" : "Login"}</button>
         `;
+    }
 
-        // If the token exists (logged in), logout on click; otherwise, login
+    addEventListeners() {
+        const token = this.getTokenFromURL() || localStorage.getItem('userToken');
         const button = this.shadowRoot.querySelector('#auth-btn');
-        button.addEventListener('click', () => {
+        // If the token exists (logged in), logout on click; otherwise, login
+        this.cleanup.onElement(button, 'click', () => {
             if (token) {
                 this.logout();
             } else {

components/project-options/index.js

@@ -6,6 +6,7 @@ import { stringFromDate, escapeHtml } from '/js/utils.js'
 import { renderPermissionError } from '../../utilities/renderPermissionError.js'
 import CheckPermissions from '../check-permissions/checkPermissions.js'
 import { onProjectReady } from '../../utilities/projectReady.js'
+import { CleanupRegistry } from '../../utilities/CleanupRegistry.js'
 import vault from '../../js/vault.js'
 
 /**
@@ -157,8 +158,8 @@ customElements.define('tpen-project-options', ProjectOptions)
  * @element line-annotation-link
  */
 class LineAnnotationLink extends HTMLElement {
-    /** @type {Function|null} Handler for mouseenter event */
-    _mouseEnterHandler = null
+    /** @type {CleanupRegistry} Registry for cleanup handlers */
+    cleanup = new CleanupRegistry()
 
     constructor() {
         super()
@@ -170,17 +171,20 @@ class LineAnnotationLink extends HTMLElement {
 
     connectedCallback() {
         this.render()
-        this._mouseEnterHandler = () => this.fetchCount()
-        this.shadowRoot.querySelector('a')?.addEventListener('mouseenter', this._mouseEnterHandler)
+        this.addEventListeners()
     }
 
-    disconnectedCallback() {
-        const anchor = this.shadowRoot?.querySelector('a')
-        if (anchor && this._mouseEnterHandler) {
-            anchor.removeEventListener('mouseenter', this._mouseEnterHandler)
+    addEventListeners() {
+        const anchor = this.shadowRoot.querySelector('a')
+        if (anchor) {
+            this.cleanup.onElement(anchor, 'mouseenter', () => this.fetchCount())
         }
     }
 
+    disconnectedCallback() {
+        this.cleanup.run()
+    }
+
     async fetchCount() {
         if (this._linesCount) return
         const page = { id: this._pageId, type: 'page' }

components/projects/project-header.js

@@ -4,6 +4,7 @@ import "../layer-selector/index.js"
 import "../column-selector/index.js"
 import CheckPermissions from "../check-permissions/checkPermissions.js"
 import { CleanupRegistry } from '../../utilities/CleanupRegistry.js'
+import { onProjectReady } from '../../utilities/projectReady.js'
 
 /**
  * ProjectHeader - Navigation header for transcription interface with canvas selection.
@@ -15,6 +16,8 @@ export default class ProjectHeader extends HTMLElement {
 
     /** @type {CleanupRegistry} Registry for cleanup handlers */
     cleanup = new CleanupRegistry()
+    /** @type {Function|null} Unsubscribe function for project ready listener */
+    _unsubProject = null
 
     constructor() {
         super()
@@ -60,14 +63,11 @@ export default class ProjectHeader extends HTMLElement {
             this.loadFailed = true
             this.render()
         })
-        this.cleanup.onEvent(eventDispatcher, 'tpen-project-loaded', this.authgate.bind(this))
-
-        if(TPEN.activeProject?._createdAt){
-            this.authgate()
-        }
+        this._unsubProject = onProjectReady(this, this.authgate)
     }
 
     disconnectedCallback() {
+        try { this._unsubProject?.() } catch {}
         this.cleanup.run()
     }
 

components/quicktype/quicktype.js

@@ -1,5 +1,6 @@
 import TPEN from "../../api/TPEN.js"
 import { CleanupRegistry } from '../../utilities/CleanupRegistry.js'
+import { onProjectReady } from '../../utilities/projectReady.js'
 
 const eventDispatcher = TPEN.eventDispatcher
 
@@ -10,6 +11,8 @@ const eventDispatcher = TPEN.eventDispatcher
 class TpenQuickType extends HTMLElement {
   /** @type {CleanupRegistry} Registry for cleanup handlers */
   cleanup = new CleanupRegistry()
+  /** @type {Function|null} Unsubscribe function for project ready listener */
+  _unsubProject = null
 
   constructor() {
     super()
@@ -67,15 +70,13 @@ class TpenQuickType extends HTMLElement {
 
   connectedCallback() {
     TPEN.attachAuthentication(this)
-    this.cleanup.onEvent(eventDispatcher, "tpen-project-loaded", () => this.loadQuickType())
+    this._unsubProject = onProjectReady(this, () => this.loadQuickType())
     this.render()
     this.addEventListeners()
-    if (TPEN.activeProject) {
-      this.loadQuickType()
-    }
   }
 
   disconnectedCallback() {
+    try { this._unsubProject?.() } catch {}
     this.cleanup.run()
   }
 

components/simple-transcription/index.js

@@ -530,12 +530,8 @@ export default class SimpleTranscriptionInterface extends HTMLElement {
       const imgBottom = this.shadowRoot.querySelector('#imgBottom img')
 
       if (imgTop && imgBottom) {
-        const onLoad = () => {
-          // Update lines once image is loaded
-          this.updateLines()
-        }
-
-        imgTop.addEventListener('load', onLoad, { once: true })
+        // Use cleanup registry for consistency, even for one-time listeners
+        this.cleanup.onElement(imgTop, 'load', () => this.updateLines(), { once: true })
         imgTop.src = imageResource
         imgBottom.src = imageResource
       }

components/transcription-block/index.js

@@ -108,23 +108,23 @@ export default class TranscriptionBlock extends HTMLElement {
         const nextPageButton = this.shadowRoot.querySelector('.next-page-button')
         const inputField = this.shadowRoot.querySelector('.transcription-input')
 
-        // Shadow DOM element listeners (cleaned up when shadow DOM is replaced)
+        // Shadow DOM element listeners
         if (prevButton) {
-            prevButton.addEventListener('click', this.moveToPreviousLine.bind(this))
+            this.cleanup.onElement(prevButton, 'click', this.moveToPreviousLine.bind(this))
         }
         if (prevPageButton) {
-            prevPageButton.addEventListener('click', () => this.navigateToPage('prev'))
+            this.cleanup.onElement(prevPageButton, 'click', () => this.navigateToPage('prev'))
         }
         if (nextButton) {
-            nextButton.addEventListener('click', this.moveToNextLine.bind(this))
+            this.cleanup.onElement(nextButton, 'click', this.moveToNextLine.bind(this))
         }
         if (nextPageButton) {
-            nextPageButton.addEventListener('click', () => this.navigateToPage('next'))
+            this.cleanup.onElement(nextPageButton, 'click', () => this.navigateToPage('next'))
         }
         if (inputField) {
-            inputField.addEventListener('blur', () => this.checkDirtyLines())
-            inputField.addEventListener('keydown', (e) => this.handleKeydown(e))
-            inputField.addEventListener('input', () => {
+            this.cleanup.onElement(inputField, 'blur', () => this.checkDirtyLines())
+            this.cleanup.onElement(inputField, 'keydown', (e) => this.handleKeydown(e))
+            this.cleanup.onElement(inputField, 'input', () => {
                 this.#transcriptions[TPEN.activeLineIndex] = inputField.value ?? ''
                 this.markLineDirty(TPEN.activeLineIndex)
                 this.persistDraft(TPEN.activeLineIndex)