diff --git a/chat-summaries/2026-08-28_01-08-thermo-nuclear-review-summary.md b/chat-summaries/2026-08-28_01-08-thermo-nuclear-review-summary.md new file mode 100644 index 0000000..89ba63f --- /dev/null +++ b/chat-summaries/2026-08-28_01-08-thermo-nuclear-review-summary.md @@ -0,0 +1,51 @@ +# Thermo-Nuclear Code Quality Review — Reddit Tweaks Extension + +**Date:** 2026-08-28 01:08 +**Task:** Deep code quality audit of the full codebase using thermo-nuclear review criteria + +## Verdict + +Not approved — several structural regressions and missed simplification opportunities identified. + +## Key Findings + +### Critical + +1. **~200 lines of CSS duplicated** between `content/styles/badges.css` (lines 409–611) and `popup/popup.css` (lines 287–489). Color picker widget and preset badge styles are copy-pasted verbatim. Fix: extract shared `color-picker.css`. + +2. **`badges.css` (611 lines) conflates three concerns** — badge styling (~85 lines), color popover chrome (~230 lines), and color picker widget (~150 lines). Should be decomposed into separate files. + +### High + +3. **Preset management logic duplicated** across `popup.js` (`renderPopupPresets`, `addPopupPreset`, `deletePopupPreset`) and `color-popover.js` (`renderPresets`, `saveAsPreset`, `removePreset`). Nearly identical CRUD pattern. Fix: extract shared `presets.js` module. + +4. **`colorToHex` identity function duplicated** in both `popup.js` and `color-popover.js`. + +### Medium + +5. **Dark mode class management scattered** across 4 files with inconsistent scoping — `cards.js` targets ``, `badges.js` targets each container, `color-popover.js` targets itself. Code-judo: apply `rt-dark` once on `` and let CSS cascade handle the rest. + +6. **DOM expando properties** (`entry._colorState`, `entry._nameInput`, etc.) used as state management in `popup.js`. Should use plain objects. + +7. **Stale closure** in `startPostObserver` — line 47 uses captured `settings` instead of freshly-loaded `updated`. + +### Low + +8. **Dead exports** (`isDarkMode`, `getColorPrefix`) on `badgeTweak` object. +9. **Verbose storage change listener** — four repetitive conditions should be data-driven. +10. **`getHelpers()` wrapper** in `color-popover.js` adds pointless indirection. + +## Recommended Priority Order + +1. Extract shared `color-picker.css` (~200 lines deleted) +2. Split `badges.css` into badges, popover, picker +3. Extract shared preset module +4. Unify dark mode to single `html.rt-dark` class +5. Fix stale closure bug +6. Clean up dead exports, verbose conditions, and unnecessary wrappers + +## Lessons Learned + +- The thermo-nuclear review skill is useful for catching CSS duplication that would otherwise grow silently — CSS files don't get the same refactoring attention as JS. +- Browser extension architecture (content scripts vs popup) makes code sharing harder since there's no module system, but shared files loaded via both manifest.json and popup.html ` + diff --git a/popup/popup.js b/popup/popup.js index f41fcee..13c1715 100644 --- a/popup/popup.js +++ b/popup/popup.js @@ -4,6 +4,7 @@ var picker = null; var activeSwatchEl = null; var saveDebounceTimer = null; + var subredditEntries = []; var COLOR_KEYS = [ "light.subredditBgColor", @@ -20,11 +21,6 @@ "dark.commentsTextColor", ]; - function colorToHex(val) { - if (!val) return "#ffffff"; - return val; - } - function getColorValue(el) { return el.dataset.value || "#ffffff"; } @@ -78,7 +74,7 @@ COLOR_KEYS.forEach(function (key) { var el = document.getElementById(key); - if (el) setColorValue(el, colorToHex(settings[key])); + if (el) setColorValue(el, settings[key] || "#ffffff"); }); renderSubredditOverrides(settings.subredditColors || {}); @@ -87,6 +83,7 @@ function renderSubredditOverrides(overrides) { var list = document.getElementById("subredditList"); list.innerHTML = ""; + subredditEntries = []; Object.entries(overrides).forEach(function (pair) { addSubredditEntry(pair[0], pair[1]); @@ -95,8 +92,8 @@ function addSubredditEntry(name, modeColors) { var list = document.getElementById("subredditList"); - var entry = document.createElement("div"); - entry.className = "subreddit-entry"; + var el = document.createElement("div"); + el.className = "subreddit-entry"; var header = document.createElement("div"); header.className = "subreddit-entry-header"; @@ -106,17 +103,7 @@ nameInput.placeholder = "subreddit"; nameInput.value = name || ""; - var removeBtn = document.createElement("button"); - removeBtn.className = "btn-remove"; - removeBtn.textContent = "\u00d7"; - removeBtn.addEventListener("click", function () { - closePicker(); - entry.remove(); - saveFromControls(); - }); - header.appendChild(nameInput); - header.appendChild(removeBtn); var modeRow = document.createElement("div"); modeRow.className = "subreddit-entry-mode"; @@ -134,39 +121,59 @@ var bgSwatch = document.createElement("div"); bgSwatch.className = "rt-color-swatch"; - setColorValue(bgSwatch, colorToHex(lightColors.bg)); + setColorValue(bgSwatch, lightColors.bg || "#ffffff"); bgSwatch.title = "Background"; var borderSwatch = document.createElement("div"); borderSwatch.className = "rt-color-swatch"; - setColorValue(borderSwatch, colorToHex(lightColors.border)); + setColorValue(borderSwatch, lightColors.border || "#ffffff"); borderSwatch.title = "Border"; var textSwatch = document.createElement("div"); textSwatch.className = "rt-color-swatch"; - setColorValue(textSwatch, colorToHex(lightColors.text)); + setColorValue(textSwatch, lightColors.text || "#ffffff"); textSwatch.title = "Text"; colorsDiv.appendChild(bgSwatch); colorsDiv.appendChild(borderSwatch); colorsDiv.appendChild(textSwatch); - entry._colorState = { - light: { bg: getColorValue(bgSwatch), border: getColorValue(borderSwatch), text: getColorValue(textSwatch) }, - dark: { bg: colorToHex(darkColors.bg), border: colorToHex(darkColors.border), text: colorToHex(darkColors.text) }, + var state = { + nameInput: nameInput, + bgSwatch: bgSwatch, + borderSwatch: borderSwatch, + textSwatch: textSwatch, + modeSelect: modeSelect, + colorState: { + light: { bg: getColorValue(bgSwatch), border: getColorValue(borderSwatch), text: getColorValue(textSwatch) }, + dark: { bg: darkColors.bg || "#ffffff", border: darkColors.border || "#ffffff", text: darkColors.text || "#ffffff" }, + }, + hadOverride: { + light: !!(modeColors && modeColors.light), + dark: !!(modeColors && modeColors.dark), + }, + dirtyModes: { light: false, dark: false }, }; - entry._hadOverride = { - light: !!(modeColors && modeColors.light), - dark: !!(modeColors && modeColors.dark), - }; - entry._dirtyModes = { light: false, dark: false }; + subredditEntries.push(state); + + var removeBtn = document.createElement("button"); + removeBtn.className = "btn-remove"; + removeBtn.textContent = "\u00d7"; + removeBtn.addEventListener("click", function () { + closePicker(); + var idx = subredditEntries.indexOf(state); + if (idx !== -1) subredditEntries.splice(idx, 1); + el.remove(); + saveFromControls(); + }); + header.appendChild(removeBtn); modeSelect.addEventListener("change", function () { closePicker(); var prev = modeSelect.value === "dark" ? "light" : "dark"; - entry._dirtyModes[prev] = true; - entry._colorState[prev] = { bg: getColorValue(bgSwatch), border: getColorValue(borderSwatch), text: getColorValue(textSwatch) }; - var next = entry._colorState[modeSelect.value]; + state.dirtyModes[prev] = true; + state.colorState[prev] = { bg: getColorValue(bgSwatch), border: getColorValue(borderSwatch), text: getColorValue(textSwatch) }; + var next = state.colorState[modeSelect.value]; setColorValue(bgSwatch, next.bg); setColorValue(borderSwatch, next.border); setColorValue(textSwatch, next.text); @@ -175,7 +182,7 @@ [bgSwatch, borderSwatch, textSwatch].forEach(function (swatch) { swatch.addEventListener("click", function () { openPickerForSwatch(swatch, function () { - entry._dirtyModes[modeSelect.value] = true; + state.dirtyModes[modeSelect.value] = true; }); }); }); @@ -184,39 +191,32 @@ saveFromControls(); }); - entry.appendChild(header); - entry.appendChild(modeRow); - entry.appendChild(colorsDiv); + el.appendChild(header); + el.appendChild(modeRow); + el.appendChild(colorsDiv); - entry._nameInput = nameInput; - entry._bgSwatch = bgSwatch; - entry._borderSwatch = borderSwatch; - entry._textSwatch = textSwatch; - entry._modeSelect = modeSelect; - - list.appendChild(entry); + list.appendChild(el); } function collectSubredditOverrides() { var overrides = {}; - var entries = document.querySelectorAll(".subreddit-entry"); - entries.forEach(function (entry) { - var name = entry._nameInput.value.trim().replace(/^r\//, ""); + subredditEntries.forEach(function (state) { + var name = state.nameInput.value.trim().replace(/^r\//, ""); if (!name) return; - var currentMode = entry._modeSelect.value; - entry._colorState[currentMode] = { - bg: getColorValue(entry._bgSwatch), - border: getColorValue(entry._borderSwatch), - text: getColorValue(entry._textSwatch), + var currentMode = state.modeSelect.value; + state.colorState[currentMode] = { + bg: getColorValue(state.bgSwatch), + border: getColorValue(state.borderSwatch), + text: getColorValue(state.textSwatch), }; var entryOverrides = {}; - if (entry._hadOverride.light || entry._dirtyModes.light) { - entryOverrides.light = entry._colorState.light; + if (state.hadOverride.light || state.dirtyModes.light) { + entryOverrides.light = state.colorState.light; } - if (entry._hadOverride.dark || entry._dirtyModes.dark) { - entryOverrides.dark = entry._colorState.dark; + if (state.hadOverride.dark || state.dirtyModes.dark) { + entryOverrides.dark = state.colorState.dark; } if (entryOverrides.light || entryOverrides.dark) { @@ -227,60 +227,31 @@ } function renderPopupPresets(presets) { - var list = document.getElementById("presetList"); - list.innerHTML = ""; - for (var i = 0; i < presets.length; i++) { - (function (index) { - var p = presets[index]; - var badge = document.createElement("span"); - badge.className = "rt-preset-badge"; - badge.style.background = p.bg; - badge.style.borderColor = p.border; - badge.style.color = p.text; - badge.textContent = "Aa"; - badge.title = "Click to apply to light subreddit colors"; - badge.addEventListener("click", function () { - setColorValue(document.getElementById("light.subredditBgColor"), p.bg); - setColorValue(document.getElementById("light.subredditBorderColor"), p.border); - setColorValue(document.getElementById("light.subredditTextColor"), p.text); - closePicker(); - saveFromControls(); - }); - - var del = document.createElement("span"); - del.className = "rt-preset-delete"; - del.textContent = "\u00d7"; - del.addEventListener("click", function (e) { - e.stopPropagation(); - deletePopupPreset(index); - }); - badge.appendChild(del); - list.appendChild(badge); - })(i); - } + window.RedditTweaks.presets.render( + document.getElementById("presetList"), + presets, + function (p) { + setColorValue(document.getElementById("light.subredditBgColor"), p.bg); + setColorValue(document.getElementById("light.subredditBorderColor"), p.border); + setColorValue(document.getElementById("light.subredditTextColor"), p.text); + closePicker(); + saveFromControls(); + }, + function (index) { + window.RedditTweaks.presets.delete(index, renderPopupPresets); + } + ); } - async function addPopupPreset() { - var preset = { - bg: getColorValue(document.getElementById("light.subredditBgColor")), - border: getColorValue(document.getElementById("light.subredditBorderColor")), - text: getColorValue(document.getElementById("light.subredditTextColor")), - }; - var settings = await window.RedditTweaks.loadSettings(); - var presets = settings.colorPresets || []; - presets.push(preset); - settings.colorPresets = presets; - await window.RedditTweaks.saveSettings(settings); - renderPopupPresets(presets); - } - - async function deletePopupPreset(index) { - var settings = await window.RedditTweaks.loadSettings(); - var presets = settings.colorPresets || []; - presets.splice(index, 1); - settings.colorPresets = presets; - await window.RedditTweaks.saveSettings(settings); - renderPopupPresets(presets); + function addPopupPreset() { + window.RedditTweaks.presets.save( + { + bg: getColorValue(document.getElementById("light.subredditBgColor")), + border: getColorValue(document.getElementById("light.subredditBorderColor")), + text: getColorValue(document.getElementById("light.subredditTextColor")), + }, + renderPopupPresets + ); } async function saveFromControls() {