Tooltip and Element creation refactoring

This change started as a refactor of the Tooltip class and turned into a complete overhaul of element building. The previous Tooltip implementation worked by storing tooltip properties (including the stringified tooltip HTML) as element attributes, then retrieving those and reinterpreting the HTML string when going to display the tooltip. This change instead only adds a single attribute to the element - a tooltip id that is used as a dictionary key pointing to a TooltipEntry. The downside of this approach is that as elements are added and removed from the DOM, the dictionary can grow very quickly and start containing entries to elements that will never be reattached to the document. To get around this, keep a weak reference of the target element, and do a sweep every 60 seconds to remove any entries for nodes that have been garbage collected. A tangential change to this was to remove usage of HTML strings in tooltips, instead requiring actual HTMLElements (or plain text strings). To help with this, a TooltipBuilder class was created to build up lines of content instead of various " += `someText<br>`" patterns. This ballooned into a far larger change that completely overhauls the element building pattern. Instead of a single buildNode method, create helpers for each element type. This allows for specializing the parameters, reducing the need to pass in empty/null arguments into buildNode directly.
danrahn · Oct 20, 2024 · deaae37 · deaae37
1 parent dbf9abb
commit deaae37
Show file tree

Hide file tree

Showing 48 changed files with 1,594 additions and 1,196 deletions.
diff --git a/Client/Script/AnimationHelpers.js b/Client/Script/AnimationHelpers.js
@@ -1,5 +1,5 @@
 import { ConsoleLog, ContextualLog } from '/Shared/ConsoleLog.js';
-import { $$ } from './Common.js';
+import { $$ } from './HtmlHelpers.js';
 import { Attributes } from './DataAttributes.js';
 
 const Log = ContextualLog.Create('Animate');

diff --git a/Client/Script/BulkActionCommon.js b/Client/Script/BulkActionCommon.js
@@ -1,4 +1,5 @@
-import { $, $$, appendChildren, buildNode, clickOnEnterCallback, ctrlOrMeta, scrollAndFocus, toggleVisibility } from './Common.js';
+import { $, $$, $append, $div, $label, $option, $plainDivHolder, $select, $span, $table, $tbody, $thead } from './HtmlHelpers.js';
+import { clickOnEnterCallback, ctrlOrMeta, scrollAndFocus, toggleVisibility } from './Common.js';
 import { ContextualLog } from '/Shared/ConsoleLog.js';
 
 import { customCheckbox } from './CommonUI.js';
@@ -156,7 +157,7 @@ class BulkActionRow {
         {},
         { thisArg : this });
 
-        return appendChildren(checkbox, buildNode('label', { for : checkboxName, class : 'hidden' }, `Marker ${identifier} Checkbox`));
+        return $append(checkbox, $label(`Marker ${identifier} Checkbox`, checkboxName, { class : 'hidden' }));
     }
 }
 
@@ -216,7 +217,7 @@ class BulkActionTable {
     }
 
     /**
-     * Retrieve the HTML <table> */
+     * Retrieve the HTML table */
     html() {
         if (this.#tbody) {
             this.#html.appendChild(this.#tbody);
@@ -262,7 +263,7 @@ class BulkActionTable {
             !this.#html,
             `BulkActionTable.buildTableHead: We should only be building a table header if the table doesn't already exist!`);
 
-        this.#html = buildNode('table', { class : 'markerTable', id : 'bulkActionCustomizeTable' });
+        this.#html = $table({ class : 'markerTable', id : 'bulkActionCustomizeTable' });
         const mainCheckbox = customCheckbox({
             title : 'Select/unselect all',
             name : 'selAllCheck',
@@ -272,9 +273,8 @@ class BulkActionTable {
         { change : BulkActionCommon.selectUnselectAll,
           keydown : [ clickOnEnterCallback, this.#onMainCheckboxKeydown.bind(this) ] });
 
-        this.#html.appendChild(
-            appendChildren(buildNode('thead'), TableElements.rawTableRow(mainCheckbox, ...columns)));
-        this.#tbody = buildNode('tbody');
+        this.#html.appendChild($thead(TableElements.rawTableRow(mainCheckbox, ...columns)));
+        this.#tbody = $tbody();
     }
 
     #onMainCheckboxKeydown(e) {
@@ -351,8 +351,8 @@ class BulkActionTable {
      * If the first item is not in the viewport, pin it to the top/bottom. */
     #repositionMultiSelectCheckboxes() {
         if (!this.#multiSelectContainer) {
-            this.#multiSelectContainer = buildNode('div', { class : 'multiSelectContainer hidden' });
-            const label = buildNode('span', { class : 'multiSelectLabel' });
+            this.#multiSelectContainer = $div({ class : 'multiSelectContainer hidden' });
+            const label = $span(null, { class : 'multiSelectLabel' });
             this.#multiSelectContainer.appendChild(label);
             let checked = true;
             for (const id of ['multiSelectSelect', 'multiSelectDeselect']) {
@@ -611,20 +611,17 @@ class BulkActionCommon {
      * @param {() => void} callback The function to call when the value changes.
      * @param {number} initialValue The initial value to select in the dropdown. Defaults to 'All'. */
     static markerSelectType(label, callback, initialValue=MarkerEnum.All) {
-        const select = appendChildren(
-            buildNode('select', { id : 'markerTypeSelect' }, 0, { change : callback }),
-            buildNode('option', { value : MarkerEnum.All }, 'All'),
-            buildNode('option', { value : MarkerEnum.Intro }, 'Intro'),
-            buildNode('option', { value : MarkerEnum.Credits }, 'Credits'),
-            buildNode('option', { value : MarkerEnum.Ad }, 'Ad'),
+        const select = $append(
+            $select('markerTypeSelect', callback),
+            $option('All', MarkerEnum.All),
+            $option('Intro', MarkerEnum.Intro),
+            $option('Credits', MarkerEnum.Credits),
+            $option('Ad', MarkerEnum.Ad),
         );
 
         select.value = initialValue;
 
-        return appendChildren(buildNode('div'),
-            buildNode('label', { for : 'markerTypeSelect' }, label),
-            select
-        );
+        return $plainDivHolder($label(label, 'markerTypeSelect'), select);
     }
 }
 

diff --git a/Client/Script/BulkAddOverlay.js b/Client/Script/BulkAddOverlay.js
diff --git a/Client/Script/BulkDeleteOverlay.js b/Client/Script/BulkDeleteOverlay.js
@@ -1,4 +1,5 @@
-import { $, appendChildren, buildNode, pad0, toggleVisibility } from './Common.js';
+import { $, $append, $br, $div, $divHolder, $h, $hr, $span, $text } from './HtmlHelpers.js';
+import { pad0, toggleVisibility } from './Common.js';
 
 import { BulkActionCommon, BulkActionRow, BulkActionTable, BulkActionType } from './BulkActionCommon.js';
 import { BulkDeleteStickySettings } from 'StickySettings';
@@ -43,14 +44,16 @@ class BulkDeleteOverlay {
      * Launch the bulk delete overlay.
      * @param {HTMLElement} focusBack The element to set focus back to after the bulk overlay is dismissed. */
     show(focusBack) {
-        const container = buildNode('div', { id : 'bulkActionContainer' });
-        const title = buildNode('h1', {}, `Delete All Markers`);
-        appendChildren(container,
+        const container = $div({ id : 'bulkActionContainer' });
+        const title = $h(1, `Delete All Markers`);
+        $append(container,
             title,
-            buildNode('hr'),
-            buildNode('h4', {}, `Are you sure you want to bulk delete markers for ${this.#mediaItem.title}?<br>This cannot be undone.`),
+            $hr(),
+            $h(4, $append($span(),
+                $text(`Are you sure you want to bulk delete markers for ${this.#mediaItem.title}?`), $br(),
+                $text(`This cannot be undone.`))),
             BulkActionCommon.markerSelectType('Delete Marker Type(s): ', this.#onApplyToChanged.bind(this), this.#stickySettings.applyTo()),
-            appendChildren(buildNode('div', { id : 'bulkActionButtons' }),
+            $divHolder({ id : 'bulkActionButtons' },
                 ButtonCreator.fullButton('Delete All',
                     Icons.Confirm,
                     ThemeColors.Green,

diff --git a/Client/Script/BulkShiftOverlay.js b/Client/Script/BulkShiftOverlay.js
@@ -1,4 +1,5 @@
-import { $, appendChildren, buildNode, msToHms, pad0, timeInputShortcutHandler, timeToMs, toggleVisibility } from './Common.js';
+import { $, $append, $br, $div, $divHolder, $h, $hr, $label, $textInput, $textSpan } from './HtmlHelpers.js';
+import { msToHms, pad0, timeInputShortcutHandler, timeToMs, toggleVisibility } from './Common.js';
 
 import { BulkActionCommon, BulkActionRow, BulkActionTable, BulkActionType } from './BulkActionCommon.js';
 import { Attributes } from './DataAttributes.js';
@@ -14,6 +15,7 @@ import { ServerCommands } from './Commands.js';
 import TableElements from './TableElements.js';
 import { ThemeColors } from './ThemeColors.js';
 import Tooltip from './Tooltip.js';
+import TooltipBuilder from './TooltipBuilder.js';
 
 /** @typedef {!import('/Shared/PlexTypes').EpisodeData} EpisodeData */
 /** @typedef {!import('/Shared/PlexTypes').SeasonData} SeasonData */
@@ -82,50 +84,35 @@ class BulkShiftOverlay {
      * Launch the bulk shift overlay.
      * @param {HTMLElement} focusBack The element to set focus back to after the bulk overlay is dismissed. */
     show(focusBack) {
-        const container = buildNode('div', { id : 'bulkActionContainer' });
-        const title = buildNode('h1', {}, `Shift Markers for ${this.#mediaItem.title}`);
-        this.#startTimeInput = buildNode(
-            'input', {
-                type : 'text',
-                placeholder : 'ms or mm:ss[.000]',
-                name : 'shiftStartTime',
-                id : 'shiftStartTime'
-            },
-            0,
-            { keyup : this.#onTimeShiftChange.bind(this),
-              keydown : timeInputShortcutHandler
-            });
+        const container = $div({ id : 'bulkActionContainer' });
+        const title = $h(1, `Shift Markers for ${this.#mediaItem.title}`);
+        this.#startTimeInput = $textInput(
+            { placeholder : 'ms or mm:ss[.000]', name : 'shiftStartTime', id : 'shiftStartTime' },
+            { keyup : this.#onTimeShiftChange.bind(this), keydown : timeInputShortcutHandler });
 
         const endVisible = this.#stickySettings.separateShift();
-        this.#endTimeInput = buildNode('input',
-            {   type : 'text',
-                placeholder : 'ms or mm:ss[.000]',
-                name : 'shiftEndTime',
-                id : 'shiftEndTime',
-                class : endVisible ? '' : 'hidden' },
-            0,
-            { keyup : this.#onTimeShiftChange.bind(this),
-              keydown : timeInputShortcutHandler
-            });
+        this.#endTimeInput = $textInput(
+            { placeholder : 'ms or mm:ss[.000]', name : 'shiftEndTime', id : 'shiftEndTime', class : endVisible ? '' : 'hidden' },
+            { keyup : this.#onTimeShiftChange.bind(this), keydown : timeInputShortcutHandler });
 
         const separateShiftCheckbox = customCheckbox(
             { id : 'separateShiftCheck', checked : endVisible },
             { change : this.#onSeparateShiftChange },
             {},
             { thisArg : this });
-        appendChildren(container,
+        $append(container,
             title,
-            buildNode('hr'),
-            appendChildren(buildNode('div', { id : 'shiftZone' }),
-                buildNode('label', { for : 'shiftStartTime', id : 'shiftStartTimeLabel' }, 'Time shift: '),
+            $hr(),
+            $divHolder({ id : 'shiftZone' },
+                $label('Time shift: ', 'shiftStartTime', { id : 'shiftStartTimeLabel' }),
                 this.#startTimeInput,
-                buildNode('label', { for : 'shiftEndTime', class : endVisible ? '' : 'hidden', id : 'shiftEndTimeLabel' }, 'End shift: '),
+                $label('End shift: ', 'shiftEndTime', { class : endVisible ? '' : 'hidden', id : 'shiftEndTimeLabel' }),
                 this.#endTimeInput),
-            appendChildren(buildNode('div', { id : 'expandShrinkCheck' }),
-                buildNode('label', { for : 'separateShiftCheck' }, 'Shift start and end times separately:'),
+            $divHolder({ id : 'expandShrinkCheck' },
+                $label('Shift start and end times separately:', 'separateShiftCheck'),
                 separateShiftCheckbox),
             BulkActionCommon.markerSelectType('Shift Marker Type(s): ', this.#onApplyToChanged.bind(this), this.#stickySettings.applyTo()),
-            appendChildren(buildNode('div', { id : 'bulkActionButtons' }),
+            $divHolder({ id : 'bulkActionButtons' },
                 ButtonCreator.fullButton('Apply',
                     Icons.Confirm,
                     ThemeColors.Green,
@@ -223,13 +210,15 @@ class BulkShiftOverlay {
         unresolved : 'Some episodes have multiple markers, please resolve below or Force Apply.',
         unresolvedAgain : 'Are you sure you want to shift markers with unresolved conflicts? Anything unchecked will not be shifted.',
         cutoff : 'The current shift will cut off some markers. Are you sure you want to continue?',
-        error : 'The current shift completely moves at least one selected marker beyond the bounds of the episode.<br>' +
-                'Do you want to ignore those and continue?',
-        unresolvedPlus : 'Are you sure you want to shift markers with unresolved conflicts? Anything unchecked will not be shifted.<br>' +
-                         'Additionally, some markers are either cut off or completely beyond the bounds of an episode (or both).<br>' +
-                         'Cut off markers will be applied and invalid markers will be ignored.',
-        cutoffPlus : 'The current shift will cut off some markers, and ignore markers beyond the bounds of the episode.<br>' +
-                     'Are you sure you want to continue?',
+        error : $textSpan('The current shift completely moves at least one selected marker beyond the bounds of the episode.', $br(),
+            'Do you want to ignore those and continue?'),
+        unresolvedPlus : $textSpan(
+            'Are you sure you want to shift markers with unresolved conflicts? Anything unchecked will not be shifted.', $br(),
+            'Additionally, some markers are either cut off or completely beyond the bounds of an episode (or both).', $br(),
+            'Cut off markers will be applied and invalid markers will be ignored.'),
+        cutoffPlus : $textSpan(
+            'The current shift will cut off some markers, and ignore markers beyond the bounds of the episode.', $br(),
+            'Are you sure you want to continue?'),
         invalidOffset : `Couldn't parse time offset, make sure it's valid.`
     };
 
@@ -247,8 +236,8 @@ class BulkShiftOverlay {
         const attributes = { id : 'resolveShiftMessage', [Attributes.BulkShiftResolveMessage] : messageType };
         let node;
         if (addForceButton) {
-            node = appendChildren(buildNode('div', attributes),
-                buildNode('h4', {}, message),
+            node = $divHolder(attributes,
+                $h(4, message),
                 ButtonCreator.fullButton(
                     'Force shift',
                     Icons.Confirm,
@@ -257,7 +246,7 @@ class BulkShiftOverlay {
                     { id : 'shiftForceApplySub', class : 'shiftForceApply' })
             );
         } else {
-            node = buildNode('h4', attributes, message);
+            node = $h(4, message, attributes);
         }
 
         const container = $('#bulkActionContainer');
@@ -778,37 +767,37 @@ class BulkShiftRow extends BulkActionRow {
             return false;
         }
 
-        let startWarnText = '';
-        let endWarnText = '';
+        const startWarnText = new TooltipBuilder();
+        const endWarnText = new TooltipBuilder();
         if (start < 0) {
-            startWarnText = `<br>This shift will truncate the marker by ${msToHms(-start)}.`;
-            BulkShiftClasses.set(newStartNode, BulkShiftClasses.Type.Warn, true, startWarnText.substring(4));
+            startWarnText.addLine(`This shift will truncate the marker by ${msToHms(-start)}.`);
+            BulkShiftClasses.set(newStartNode, BulkShiftClasses.Type.Warn, true, startWarnText.get());
             this.#isWarn = true;
         } else {
             BulkShiftClasses.set(newStartNode, BulkShiftClasses.Type.On, true);
         }
 
         if (end > maxDuration) {
             this.#isWarn = true;
-            endWarnText = `<br>This shift will truncate the marker by ${msToHms(end - maxDuration)}.`;
-            BulkShiftClasses.set(newEndNode, BulkShiftClasses.Type.Warn, true, endWarnText.substring(4));
+            endWarnText.addLine(`This shift will truncate the marker by ${msToHms(end - maxDuration)}.`);
+            BulkShiftClasses.set(newEndNode, BulkShiftClasses.Type.Warn, true, endWarnText.get());
         } else {
             BulkShiftClasses.set(newEndNode, BulkShiftClasses.Type.On, true);
         }
 
         const overlap = this.#parent.overlappingMarkers(this.markerId(), this.#episode.metadataId, newStart, newEnd);
         for (const marker of overlap) {
-            const warnText = `<br>Overlaps with ${marker.markerType} marker: [${msToHms(marker.start)}-${msToHms(marker.end)}]`;
-            startWarnText += warnText;
-            endWarnText += warnText;
+            const warnText = `Overlaps with ${marker.markerType} marker: [${msToHms(marker.start)}-${msToHms(marker.end)}]`;
+            startWarnText.addLine(warnText);
+            endWarnText.addLine(warnText);
         }
 
-        if (startWarnText) {
-            BulkShiftClasses.set(newStartNode, BulkShiftClasses.Type.Warn, true, startWarnText.substring(4));
+        if (!startWarnText.empty()) {
+            BulkShiftClasses.set(newStartNode, BulkShiftClasses.Type.Warn, true, startWarnText.get());
         }
 
-        if (endWarnText) {
-            BulkShiftClasses.set(newEndNode, BulkShiftClasses.Type.Warn, true, endWarnText.substring(4));
+        if (!endWarnText.empty()) {
+            BulkShiftClasses.set(newEndNode, BulkShiftClasses.Type.Warn, true, endWarnText.get());
         }
 
         return true;

diff --git a/Client/Script/ButtonCreator.js b/Client/Script/ButtonCreator.js
@@ -1,5 +1,6 @@
-import { $, $$, addEventsToElement, appendChildren, buildNode, toggleVisibility } from './Common.js';
+import { $, $$, $append, $div, $span, addEventsToElement  } from './HtmlHelpers.js';
 import { ContextualLog } from '/Shared/ConsoleLog.js';
+import { toggleVisibility } from './Common.js';
 
 import { addWindowResizedListener, isSmallScreen } from './WindowResizeEventHandler.js';
 import { Attributes } from './DataAttributes.js';
@@ -55,9 +56,9 @@ class ButtonCreator {
      * @param {AttributeMap} attributes Additional attributes to set on the button. */
     static fullButton(text, icon, color, clickHandler, attributes={}) {
         const button = ButtonCreator.#tableButtonHolder('buttonIconAndText', clickHandler, attributes);
-        return appendChildren(button,
+        return $append(button,
             getSvgIcon(icon, color),
-            buildNode('span', { class : 'buttonText' }, text));
+            $span(text, { class : 'buttonText' }));
     }
 
     /**
@@ -102,7 +103,7 @@ class ButtonCreator {
         }
 
         const button = ButtonCreator.#tableButtonHolder('buttonIconOnly', clickHandler, attributes);
-        return appendChildren(button, getSvgIcon(icon, color));
+        return $append(button, getSvgIcon(icon, color));
     }
 
     /**
@@ -112,7 +113,7 @@ class ButtonCreator {
      * @param {AttributeMap} [attributes={}] Additional attributes to set on the button. */
     static textButton(text, clickHandler, attributes={}) {
         const button = ButtonCreator.#tableButtonHolder('buttonTextOnly', clickHandler, attributes);
-        return appendChildren(button, buildNode('span', { class : 'buttonText' }, text));
+        return $append(button, $span(text, { class : 'buttonText' }));
     }
 
     /**
@@ -171,8 +172,7 @@ class ButtonCreator {
      * @param {EventListener} clickHandler The callback function when the button is clicked.
      * @param {AttributeMap} attributes Additional attributes to set on the button. */
     static #tableButtonHolder(className, clickHandler, attributes) {
-        const button = buildNode(
-            'div',
+        const button = $div(
             { class : `button noSelect ${className}`, tabindex : '0' },
             0,
             { keydown : ButtonCreator.#tableButtonKeydown });