fix: address code review findings — DRY delta-pill, var→const/let, tokenize colors

public/css/shared.css

@@ -662,7 +662,7 @@ a:hover {
 }
 
 .delta-pill.flat {
-    background: oklch(0.95 0.004 260);
+    background: var(--paper-2);
     color: var(--ink-3);
 }
 

public/js/chart.js

@@ -96,7 +96,7 @@ function createRatingChart(container, history) {
         if (isLast) {
             svg.appendChild(el('circle', {
                 cx: p.x.toFixed(1), cy: p.y.toFixed(1),
-                r: '4', fill: 'var(--accent)', stroke: 'white', 'stroke-width': '2'
+                r: '4', fill: 'var(--accent)', stroke: 'var(--paper)', 'stroke-width': '2'
             }));
         } else {
             svg.appendChild(el('circle', {

public/js/players.js

@@ -1,6 +1,17 @@
-var cachedDebugInfo = {};
+const cachedDebugInfo = {};
-var pendingPlayerData = null;
+let pendingPlayerData = null;
-var openPdgaNumber = null;
+let openPdgaNumber = null;
+
+// ── Delta-pill helper ─────────────────────────────
+// Returns { text, cls, html } — use .text/.cls to update existing DOM elements
+// safely; .html is available for insertion contexts.
+function renderDeltaPill(value, extraClass) {
+    if (value == null) return null;
+    const cls  = value > 0 ? 'up' : value < 0 ? 'down' : 'flat';
+    const text = value > 0 ? '+' + value : String(value);
+    const extra = extraClass ? ' ' + extraClass : '';
+    return { text, cls, html: `<span class="delta-pill ${cls}${extra}">${text}</span>` };
+}
 
 function setupTooltipsAfterSwap() {
     document.body.addEventListener('htmx:afterSwap', function(event) {
@@ -49,16 +60,16 @@ function initRatingsTooltips() {
 }
 
 function togglePlayerHistory(pdgaNumber) {
-    var historyRow = document.getElementById('history-' + pdgaNumber);
+    const historyRow = document.getElementById('history-' + pdgaNumber);
-    var contentDiv = document.getElementById('history-content-' + pdgaNumber);
+    const contentDiv = document.getElementById('history-content-' + pdgaNumber);
-    var expandableRow = document.getElementById('row-' + pdgaNumber);
+    const expandableRow = document.getElementById('row-' + pdgaNumber);
 
-    var isOpen = historyRow.style.display === 'table-row';
+    const isOpen = historyRow.style.display === 'table-row';
 
     // Close any previously-open row
     if (openPdgaNumber !== null && openPdgaNumber !== pdgaNumber) {
-        var prevHistory = document.getElementById('history-' + openPdgaNumber);
+        const prevHistory = document.getElementById('history-' + openPdgaNumber);
-        var prevRow = document.getElementById('row-' + openPdgaNumber);
+        const prevRow = document.getElementById('row-' + openPdgaNumber);
         if (prevHistory) {
             prevHistory.style.display = 'none';
             prevHistory.classList.remove('is-open');
@@ -144,11 +155,9 @@ async function refreshPlayer(pdgaNumber) {
 
             const deltaMonthPill = ratingCell.querySelector('.delta-pill');
             if (deltaMonthPill && data.player.ratingChange != null) {
-                const pillChange = data.player.ratingChange;
+                const pill = renderDeltaPill(data.player.ratingChange);
-                const pillText = pillChange > 0 ? `+${pillChange}` : pillChange.toString();
+                deltaMonthPill.textContent = pill.text;
-                const pillClass = pillChange > 0 ? 'up' : pillChange < 0 ? 'down' : 'flat';
+                deltaMonthPill.className = `delta-pill ${pill.cls}`;
-                deltaMonthPill.textContent = pillText;
-                deltaMonthPill.className = `delta-pill ${pillClass}`;
             }
         }
     } catch (error) {
@@ -509,15 +518,15 @@ function closeAddPlayerModal(event) {
 
 // ── Sparkline toggle ───────────────────────────────
 document.addEventListener('DOMContentLoaded', function() {
-    var btn = document.getElementById('trendchart-toggle');
+    const btn = document.getElementById('trendchart-toggle');
     if (!btn) return;
 
-    var state = localStorage.getItem('ratingtracker.sparklines') || 'on';
+    const state = localStorage.getItem('ratingtracker.sparklines') || 'on';
     document.body.dataset.sparklines = state;
     btn.setAttribute('aria-pressed', state === 'on' ? 'true' : 'false');
 
     btn.addEventListener('click', function() {
-        var next = document.body.dataset.sparklines === 'on' ? 'off' : 'on';
+        const next = document.body.dataset.sparklines === 'on' ? 'off' : 'on';
         document.body.dataset.sparklines = next;
         btn.setAttribute('aria-pressed', next === 'on' ? 'true' : 'false');
         localStorage.setItem('ratingtracker.sparklines', next);
@@ -527,9 +536,9 @@ document.addEventListener('DOMContentLoaded', function() {
 // ── Expandable row keyboard support ───────────────
 document.addEventListener('keydown', function(e) {
     if (e.key !== 'Enter' && e.key !== ' ') return;
-    var row = e.target;
+    const row = e.target;
     if (!row.classList || !row.classList.contains('expandable-row')) return;
     e.preventDefault();
-    var pdgaNumber = row.id.replace('row-', '');
+    const pdgaNumber = row.id.replace('row-', '');
     togglePlayerHistory(parseInt(pdgaNumber, 10));
 });

views/partials/delta-pill.ejs

@@ -0,0 +1,10 @@
+<%/* delta-pill.ejs — renders a Δ-pill span.
+   Locals:
+     value      {number|null}  — the delta value
+     extraClass {string}       — optional additional CSS class (e.g. 'delta-predicted-pill')
+*/%>
+<% if (typeof value !== 'undefined' && value != null) {
+    const _cls  = value > 0 ? 'up' : value < 0 ? 'down' : 'flat';
+    const _text = value > 0 ? '+' + value : value.toString();
+    const _xtra = (typeof extraClass !== 'undefined' && extraClass) ? ' ' + extraClass : '';
+%><span class="delta-pill <%= _cls %><%= _xtra %>"><%= _text %></span><% } %>

views/partials/player-history.ejs

@@ -1,14 +1,6 @@
 <%
-var monthChange = (typeof player !== 'undefined' && player) ? player.ratingChange : null;
+const hasPlayer = (typeof player !== 'undefined' && player);
-var monthPillText = monthChange != null ? (monthChange > 0 ? '+' + monthChange : monthChange.toString()) : null;
+const chartPdgaNumber = hasPlayer ? player.pdgaNumber : pdgaNumber;
-var monthPillClass = monthChange > 0 ? 'up' : monthChange < 0 ? 'down' : 'flat';
-
-var gapPredicted = (typeof player !== 'undefined' && player) ? (player.deltaPredicted ?? null) : null;
-var gapPillText = gapPredicted != null ? (gapPredicted > 0 ? '+' + gapPredicted : gapPredicted.toString()) : null;
-var gapPillClass = gapPredicted > 0 ? 'up' : gapPredicted < 0 ? 'down' : 'flat';
-
-var hasPlayer = (typeof player !== 'undefined' && player);
-var chartPdgaNumber = hasPlayer ? player.pdgaNumber : pdgaNumber;
 %>
 <div class="player-detail">
   <% if (hasPlayer) { %>
@@ -24,9 +16,7 @@ var chartPdgaNumber = hasPlayer ? player.pdgaNumber : pdgaNumber;
     <div>
       <dt>Change vs last month</dt>
       <dd>
-        <% if (monthPillText) { %>
+        <% if (player.ratingChange != null) { %><%- include('delta-pill', { value: player.ratingChange }) %><% } else { %>—<% } %>
-        <span class="delta-pill <%= monthPillClass %>"><%= monthPillText %></span>
-        <% } else { %>—<% } %>
       </dd>
     </div>
     <div>
@@ -36,9 +26,7 @@ var chartPdgaNumber = hasPlayer ? player.pdgaNumber : pdgaNumber;
     <div>
       <dt>Gap to predicted</dt>
       <dd>
-        <% if (gapPillText) { %>
+        <% if (player.deltaPredicted != null) { %><%- include('delta-pill', { value: player.deltaPredicted, extraClass: 'delta-predicted-pill' }) %><% } else { %>—<% } %>
-        <span class="delta-pill delta-predicted-pill <%= gapPillClass %>"><%= gapPillText %></span>
-        <% } else { %>—<% } %>
       </dd>
     </div>
   </dl>

views/partials/ratings-table.ejs

@@ -43,15 +43,7 @@ function renderSparkline(values) {
         </thead>
         <tbody>
             <% ratings.forEach(function(player, index) {
-                var ratingChange = player.ratingChange;
+                const sparklineSvg = renderSparkline(player.monthlyHistory || []);
-                var ratingChangePillText = ratingChange != null ? (ratingChange > 0 ? '+' + ratingChange : ratingChange.toString()) : null;
-                var ratingChangePillClass = ratingChange > 0 ? 'up' : ratingChange < 0 ? 'down' : 'flat';
-
-                var deltaPredicted = player.deltaPredicted ?? null;
-                var deltaPredictedPillText = deltaPredicted != null ? (deltaPredicted > 0 ? '+' + deltaPredicted : deltaPredicted.toString()) : null;
-                var deltaPredictedPillClass = deltaPredicted > 0 ? 'up' : deltaPredicted < 0 ? 'down' : 'flat';
-
-                var sparklineSvg = renderSparkline(player.monthlyHistory || []);
             %>
             <tr id="row-<%= player.pdgaNumber %>" class="expandable-row" tabindex="0" onclick="togglePlayerHistory(<%= player.pdgaNumber %>)">
                 <td class="mobile-hide"><%= index + 1 %></td>
@@ -65,9 +57,7 @@ function renderSparkline(values) {
                         <span class="rating-value" data-rating="<%= player.rating || '' %>" data-stddev="<%= player.stdDev || '' %>" data-pdga="<%= player.pdgaNumber %>" style="cursor: help;"><%- player.rating || '<span style="color: var(--text-muted); font-style: italic;">Click refresh</span>' %></span>
                         <i class="fas fa-sync-alt refresh-icon" onclick="refreshPlayer(<%= player.pdgaNumber %>)" title="Refresh player data"></i>
                     </div>
-                    <% if (ratingChangePillText) { %>
+                    <%- include('delta-pill', { value: player.ratingChange }) %>
-                    <span class="delta-pill <%= ratingChangePillClass %>"><%= ratingChangePillText %></span>
-                    <% } %>
                     <% if (sparklineSvg) { %>
                     <span class="sparkline"><%- sparklineSvg %></span>
                     <% } %>
@@ -79,9 +69,7 @@ function renderSparkline(values) {
                         <i class="fas fa-question-circle debug-icon" onclick="showDebugInfo(<%= player.pdgaNumber %>)" title="Show calculation details" style="margin-left: 5px; color: var(--text-muted); cursor: pointer; opacity: 0.6;"></i>
                         <i class="fas fa-sync-alt refresh-icon" onclick="refreshRoundHistory(<%= player.pdgaNumber %>)" title="Refresh prediction data"></i>
                     </div>
-                    <% if (deltaPredictedPillText) { %>
+                    <%- include('delta-pill', { value: player.deltaPredicted, extraClass: 'delta-predicted-pill' }) %>
-                    <span class="delta-pill delta-predicted-pill <%= deltaPredictedPillClass %>"><%= deltaPredictedPillText %></span>
-                    <% } %>
                     <div class="std-dev-tooltip" id="tooltip-stddev-<%= player.pdgaNumber %>"></div>
                 </td>
             </tr>