52 lines
2.8 KiB
Markdown
52 lines
2.8 KiB
Markdown
|
# 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 `<html>`, `badges.js` targets each container, `color-popover.js` targets itself. Code-judo: apply `rt-dark` once on `<html>` 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 `<script>`/`<link>` tags work fine.
|
|||
|
|
- IIFE-scoped modules on a global namespace are a reasonable pattern for small extensions but make duplication easy to miss since there's no import graph to inspect.
|