Repository navigation
Conversation
Records the repository-wide audit (scored dimensions, hotspots, dependency/icon/CSS inventories, regression risks) and the execution plan derived from it, following documents/REFACTORING.md sections 23-24.
ChatBadges returned null before creating its memo, so the reactive owner depended on the mount-time value of hideAllBadges. Existing message rows never reacted when the setting changed; only newly mounted rows picked it up. The check now lives inside the memo.
Replaces two overlapping icon sources with one: the Hugeicons icon font (964 KB woff2 plus a 420 KB stylesheet, used through stringly-typed hgi-* classes) and lucide-solid (5 deep-subpath imports). Adds a typed Icon primitive over @hugeicons/core-free-icons (MIT, zero dependencies, per-icon subpath exports) and a PlatformGlyph module that owns the monochrome brand glyphs previously duplicated inline in setup.tsx and TwitchChannelField.tsx. Custom event, reply, role and sparkline graphics stay inline because they are bespoke visuals rather than UI icons. Removes the font assets, the index.html stylesheet link and the now-dead .hgi-stroke rule. Production dist shrinks by about 1.35 MB.
…llision root.css declared a second, parallel token set alongside the SolidCN tokens in app.css. Six of its tokens were unreferenced, --panel and --text-secondary were used only by the unused .bg-panel and .text-secondary utility classes, and --border was declared in both files with incompatible values so that the app.css definition silently won. --text stays because the overlay renders on a transparent OBS background and must not inherit the themed foreground token, which is near-black in the light theme. Verified by computed style: body color is still rgb(255,255,255) and --border resolves from app.css alone.
Deletes nine modules that nothing imported from either src or tests: the chat parsing/formatting leftovers sanitize, urlParser, markdownParser, emojiRenderer, actionMessages, userNoticeParser and chatUtils, plus the two barrels utils/chat/index.ts and utils/ui/index.ts. The utils/chat barrel existed only to re-export ChatConfig through a second hop, so the 15 files that used it now import the type from its real home in config/chatUrlParams. YouTubeChatService and types/userNotice.ts were superseded by ExternalChatService and replyThread respectively and had no remaining references. RteCosmeticsService.clear() is deliberately kept: the audit calls for that cleanup to be wired into its owning runtime rather than deleted, which is a separate behaviour change.
The Twitch-bot, YouTube-bot, Kick-bot and viewer-allowlist rows each repeated the same borderless input markup and class string. They now share components/setup/SetupChipInput, a feature-level wrapper rather than the generic Input primitive, because the field sits inside an existing bordered chip container and must stay borderless and transparent. Moves ColorPickerField under components/setup/ so feature components live with their feature. The remaining raw controls in setup.tsx are bespoke single-purpose elements with their own CSS contracts; pushing them through Button would inject conflicting base styles. Verified in a real browser: all four rows still commit a chip on Enter and clear the draft, with no console errors beyond an expected 404 from kick.com for a probe username.
renderMessageContent.ts declared codePointToCodeUnit twice with identical bodies, and escapeAttr byte-identically to its copy in emoteModifiers/html.ts. escapeAttr now has one definition, exported from the module that already owns chat HTML-string assembly, and codePointToCodeUnit is hoisted to module scope with a note on why Twitch ranges need the conversion. Behaviour preserving: tests/renderMessageContent.test.ts and tests/messageTokenSnapshot.test.ts pass unchanged, the overlay renders identically and the chat chunk is marginally smaller.
The route declared the storage key registry plus its own guarded read/write helpers, while config/setupTemplates carried a second, differently shaped copy of the same availability and try/catch logic. Both now go through services/storage/setupStorage. The adapter stays dependency-free so the config -> services edge it introduces cannot form a cycle; the alternative, relocating template persistence into services, would have been a larger move for the same result. The injectable StorageLike parameter is preserved because tests/setupTemplates.test.ts passes an in-memory store. Verified in a browser: a typed channel is persisted, survives a reload, reaches the generated overlay URL, and removing it deletes the key rather than storing an empty string.
The route declared its own Twitch and Kick bot-profile fetchers, and TwitchChannelField carried a byte-identical fetchJsonWithTimeout plus a second copy of the GQL endpoint and client id. Both now share services/network/fetchJsonWithTimeout and the constants in config/twitch. normalizeBotLogin keeps stripping a leading @ even though config/chatUrlParams parseBotNames does not, because users paste handles with the sigil while URL values never carry one; merging the two would change URL parsing. Verified in a browser: the Twitch lookup resolves 35 display names and 33 avatars for the default bot list, the Kick lookup resolves its entries, and an unknown login degrades to a fallback chip without a console error.
The route owned the Chromium-only browser sniff, the raw per-face font records and the enumeration try/catch, so the Local Font Access API surface was spread through presentation code. services/setup/localFonts now returns a typed result and the route only maps it onto its status signal. Verified in a browser: the real API yields the empty status without a console error in headless Chromium, and an injected fake reports two families with grouped, sorted styles and drops a whitespace-only family.
The coercion helpers were rebuilt inside the component on every render and one pair of them, toIntOrFalse and toSecondsOrFalse, was byte-identical. They now live in config/formValues at module scope, which is also where the form-side counterpart of the chatUrlParams ParamKind vocabulary belongs. chatUrlParams parses and serializes its intOrFalse and secondsOrFalse kinds through identical code, so the two helpers collapsed into toPositiveIntOrFalse; tests/formValues.test.ts pins that premise and characterizes the form -> URL -> config round trip.
The route owned the whole form-to-config projection and the overlay link builder, which meant the URL contract could only be exercised by driving the page. config/setupConfig now takes an explicit SetupFormState, so the projection is pure and the route only collects signals into it. buildOverlayUrl gained an optional baseUrl so the contract is assertable without a document; the preview still drops ms through the same option it used before. tests/setupConfig.test.ts pins default projection, de-channelling, clamping, malformed input, extra-parameter precedence and a full export -> parse round trip. Verified in a browser: channel, size and line-height each change the generated link and survive a reload, and the embedded preview renders demo messages with ms absent and no console errors.
The route owned the preview iframe protocol directly: the config postMessage, the session-key gate, the debounce timer, the direct src swap and its cancellation. features/setup/previewSync now owns that protocol while the route keeps only the reactive effects that read signals, and the navigation URL is built lazily so an unchanged session costs nothing. The module imports the preview message builder from its own module rather than the preview barrel, which keeps the setup feature from pulling in the emote and style modules and lets the synchronizer run under a minimal window shim. Verified by tests/previewSync.test.ts and in a browser: a committed channel change reloads the frame, a config-only change does not, and the overlay still re-renders from the pushed config.
The route inlined the document style snapshot, the lock and its restore, and the reduced-motion subscription with a manual initial read. features/setup/setupDocument now owns both: lockSetupDocument returns its own release function, and watchReducedMotion reports the current preference immediately so the caller needs no separate read. Verified in a browser: the lock applies on setup, a directly loaded /status does not touch those styles, a client-side route change releases the lock, and a reduced-motion context has the demo paused on mount.
The route owned the searchable-element selector, the section lookup, the highlight class bookkeeping, the counter format and the reveal scroll, while utils/setupSearch held the pure matcher. features/setup/settingsSearch now owns the DOM half and the route keeps only the signals and memos, passing its own scrollToSection in because the workspace scroll state belongs to it. Verified in a browser: a query matching 8 rows across 4 sections steps the counter forward and back and clearing removes every highlight and nav marker.
renderUserChip and ffzBadgeMergeBlock were closures returning JSX inside the route. They are now components/setup/UserChip and components/setup/FfzBadgeMergeBlock, and the badge preview asset URL moved with the block that uses it. UserChip takes the profile map as a getter rather than one resolved profile, because bot profiles arrive after the chip renders; roleBadgeMergeOptions stays in the route because it is the signal wiring. Verified in a browser: 29 bot chips render with 29 avatars and a resolved display name, removing one leaves 28, and the three role pills toggle their pressed state.
… owner liveRuntime.destroy now releases everything its initialize installed: the generated overlay stylesheets, the shared rte proxy flag, and the badge and emote caches, which are keyed by channel and previously outlived the runtime that filled them. badgeService.reset restores a snapshot taken at construction so the built-in fallback badges survive it, and sevenTVCosmeticsService.clearAllCaches is now reached from chatPresentationService.cleanup. Verifying that exposed two leaks from modules that appended a style element unconditionally: two #chat-animations elements existed, so removing one by id left the other, and #chat-layout was never removed at all. The three injectors now reuse an existing element, the layout and fade managers gained cleanup, and OverlayStyleManager.cleanup removes every element matching an id. A MutationObserver trace across teardown shows all five stylesheets removed and none left behind. tests/serviceReset.test.ts pins the emote and badge reset behaviour, including that the built-in fallbacks survive a reset.
…rity ChatMessage had two clearEntryAnimation closures that differed only in whether they removed the measured flow shift property, and removing a property that was never set is a no-op, so one closure now serves both branches. badgePriority moved next to ChatBadges, its only importer, with git mv. Verified in a browser by timing each row from the entry class appearing to it disappearing: flow entered 11 rows with a 495ms maximum lifetime and fade 10 rows with 335ms, and neither mode left a row holding the class or the shift property past the animation window. The layout manager half of the same step is recorded as blocked by layering: chatPresentationService owns it, and moving it under features/chat-overlay would make a service import a feature.
The four chatStyles generators emitted rules whose shape followed the size preset, and both the live overlay and the preview appended them to the document head. getOverlayStyleVariables now returns a property map the way getChatEventStyleVariables does, chat.css owns every rule, and the two runtimes write the same properties instead of each generating their own stylesheet - which also removes the preview's duplicate copy of that logic. SIZE_CONFIGS stays as the single source of truth because renderMessageContent reads it for emote geometry; it now publishes properties rather than rules. The emote scale is folded into the published emote and emoji sizes, since a stylesheet cannot multiply a preset by a runtime value. The boolean variants became :root attributes, and the gigantified width is published under a --chat- name because it is no longer scoped to one element. Verified by diffing computed styles for 20 configurations and 14 selectors before and after. The only differences are inherited gigantified-width readings with none on the element that consumes it, readings of an artificial emoji probe whose real counterpart sets the same size inline, and one container height inside the measured noise floor of two identical runs.
utils/ui/animationUtils held the animation modes, the speed bounds and the pure mappings next to functions that read and write the document, so the URL contract and the setup form depended on a DOM-touching module. The vocabulary moved to config/chatAnimation with no DOM access, and the stylesheet generation moved to services/chat/runtime/animationStyles, next to the overlay runtime that consumes it. Verified in a browser: flow and fade still animate and clean up with no rows stuck and no console error; the module test moved to tests/chatAnimation.test.ts and imports each half from its new home.
renderMessageWithEmotes imported networkClient directly, so the renderer owned network policy and could not run without infrastructure. It now takes a RenderMessageOptions object with a resolveUrl callback and imports only the NetworkRoute type, while ChatText passes the real resolver from the component layer. Two tests cover the seam: every asset url goes through the injected resolver, and a message with nothing to resolve calls it zero times. The existing suite passes the real resolver so its RTE-rewrite coverage is preserved. Verified in a browser: the dev fixture renders 4 emotes and 2 emoji with resolved CDN urls and 24 messages, with no console error.
The appearance, styling, behaviour and toggle descriptor arrays were declared inline in the route, so the route owned both the section contents and the form wiring. They now live in components/setup/sections as factories that take one source object and destructure it up front, which keeps every row body byte-identical to what the route declared and makes the extraction unable to silently change a label or a control. setup.tsx goes from 2779 lines at the start of the phase to 1909. Verified in a browser: each section renders the rows its array declares, toggling a switch changes the generated overlay url in every section that has one, and the settings search still reports 8 matches across the extracted rows with no console error.
The preview pane's controls were inline JSX whose selection thumbs were measured by route-level refs, signals and a resize observer. components/setup/PreviewControls now owns the markup and that measurement together, because the refs exist only to position the thumbs and splitting them across a fifteen-prop boundary would have spread the coordination instead of reducing it. setup.tsx goes from 2779 lines at the start of the phase to 1686. Verified in a browser: four stage swatches with the pressed state tracking the click, the custom colour picker appearing only for the custom backdrop, both thumbs measured at height 36px with translateY 40px and 0, and the pause control toggling its pressed state, with no console error.
The ui cache-boundary group still listed lucide-solid after the dependency was removed, and did not list the icon package that replaced it, so the icons were being bundled into the entry chunk instead of the group the comment describes.
Rewriting the overlay style manager to publish custom properties dropped the stylesheet teardown it used to perform, so the animation stylesheet it injects outlived the runtime. clearOverlayStyles now removes it through clearAnimationStyles, and tests/overlayStyleManager.test.ts guards both the removal and the no-accumulation case. Found by a browser lifecycle check: the running overlay now holds only the animation and layout stylesheets, which are generated by design, and leaving the route removes both and clears every published property.
Walks the section 25 checklist item by item with the evidence each item rests on, and records the closing state of the phases.
The custom wrapper traversed the icon payload, kebab-cased attributes and built the svg tags. The vendor now ships @hugeicons/solid-js, so that logic has one owner again and the local Icon keeps only the project convention: currentColor, 1em default size and decorative accessibility. The payload keeps its own stroke width because forcing one at the root would stroke a fill-only icon. Verified by diffing every svg on the setup page before and after: 65/65 identical geometry, font size, stroke width and colour, no page errors. The only attribute difference is the vendor's explicit color=currentColor on the root, which inherits to the same value. Build: ui chunk +2.77 kB, setup chunk -0.50 kB.
The detailed rules lived only in documents/REFACTORING.md, which has to be discovered manually. The root file now carries the rules an automated change can regress: layer direction, lifecycle ownership, primitive and icon boundaries, the styling and custom-property contract, accessibility and OBS constraints, public contracts, dependency and test policy. It also requires repository-wide refactor work to read both documents/REFACTORING.md and the new completion supplement, which is committed here as the plan input for that work.
The settings search imported SetupSectionId from a presentation component, which is the wrong direction for a shared contract. The identity now lives in features/setup/model/setupSections.ts, so the route, the workspace layout and the search all import the same type without importing each other; the icon and translation metadata stays with the presentation that renders it. Two pure helpers sat in JSX modules that now import the client-only vendor icon renderer, which made their tests unloadable without a DOM: - the Twitch/Kick channel search moved from components/setup/TwitchChannelField.tsx to services/setup/channelSearch.ts; - resolveSetupText and SetupText moved from SetupLayout.tsx to components/setup/setupText.ts. The move also collapsed a real duplication: normalizeLogin, the avatar fallback letter and the login-list merge existed twice, in TwitchChannelField.tsx and botProfiles.ts. They now have one owner in services/setup/logins.ts. Verified: an import-boundary sweep finds no features-to-components, features-to-routes or lower-layer-to-upper-layer edge left; typecheck, lint and 375 tests pass; in the browser the Twitch search resolves a login with its avatar and commits it to the overlay URL (c=linaryx) and the Kick search returns its suggestions, with no console error.
The constructor builds the live or preview runtime and the predictions controller, but destroy() returned immediately when start() had never run, so construction and teardown disagreed about ownership. destroy() now releases what the constructor built, is idempotent, and is terminal: a destroyed application cannot be started again, which is also what makes it safe while start() is still awaiting initialize(). tests/chatOverlayApplication.test.ts characterizes construct-destroy, double destroy, double start, start after destroy and destroy during a pending initialization.
Resetting a singleton dropped what it held but not what was already in flight: a response that arrived after the owning runtime was destroyed still committed into the store the next runtime had just cleaned. The same held for the 7TV visibility toggle, where a response loaded under the previous filter could land after the cache was cleared. Each affected service now carries a generation that reset (or, for the cosmetics caches, clearAllCaches) bumps. A load captures it on entry and skips its commit when the store moved on. Covered: the emote global and channel loaders, the badge Twitch, third-party, GQL-set and per-user loaders, the 7TV cosmetics catalog, channel and user loads, and the feature integration, which could mark itself initialized after being destroyed and then block the next runtime. tests/assetTeardownRace.test.ts holds each response open, resets, and only then resolves it: all seven tests fail against the previous code.
The preview runtime enabled the shared RTE proxy flag and filled the shared emote, badge and mention stores, but its teardown released neither, so a document that had shown a preview kept the demo's state and proxy routing after the runtime was gone. destroy() now clears the flag and the stores, and only for a runtime that actually claimed them: holdsSharedState is set by initialize()/updateConfig(), so destroying a never-initialized runtime does not disarm another owner. A destroyed runtime also ignores further configuration instead of re-injecting styles it just cleaned up. The ownership question is recorded in the code: concurrency is not possible by design, because a document hosts exactly one chat runtime, which is why a process-wide boolean is sufficient here rather than a lease or reference count. tests/previewRuntimeTeardown.test.ts covers the release, the never-claimed case and the post-destroy update. Verified in the browser that the setup preview still commits a channel, reloads the frame, picks up a config-only font change (20px to 48px) and reports no page error.
The helper wrote a literal #09090b onto html and body, which the app-wide OBS contract already overrides: app.css declares background: transparent !important for html, body and #root, and the workspace paints its own surface on .setup-root, which covers the viewport. Measured before the change: the inline colour resolved to rgba(0, 0, 0, 0) while .setup-root painted rgb(9, 9, 11). The lock now sets data-setup-document on the root and the setup stylesheet owns what that means: no document scrolling, with the height the columns rely on. The release restores exactly the attribute state it found. Verified by a DOM-level comparison rather than by screenshot: the page is not pixel-stable here (two shots of an unchanged state differ, even in a region with no animation), so a pixel hash could not prove anything. Comparing every element's rect plus the lock-relevant computed styles and every scroll height between the two implementations reports zero changes, and in the browser the lock is applied on setup, removed when navigating in-app to /status, and re-applied on a setup route, with no page error.
layoutUtils and fadeUtils are not general helpers: they publish stylesheets written against #chat_container, .chat_line and .message-fade-out, and ChatPresentationService is their only consumer. They now live in services/chat/runtime/, which already holds the chat runtime's other infrastructure, so the dependency direction is unchanged and the files sit in the layer that owns them. That also resolves the two-classes-one-name problem: the second LayoutManager in utils/ui/layoutManager.ts is deleted. Its only caller was chatFeatureIntegration.setOptions, wiring reverseLineOrder and singleChatter into it, and it was provably a no-op: setContainer was never called, so applyLayout returned before touching anything, and its shouldShowMessage, getContainerClasses, getLayoutCSS and injectCSS had no callers. The singleChatter behaviour that actually runs is ChatPresentationService's own. Verified in the browser: the preview overlay publishes #chat-layout, applies layout-vertical/layout-horizontal/layout-reverse, renders 9 messages, keeps exactly one #chat-animations stylesheet and reports no page error, in all three layout modes.
The 7TV paint stylesheet was created on demand and never removed: its owner generated rules from provider data and left the element in the document after the runtime that filled it was torn down, and the cached sheet reference would have kept writing into that detached element. clearAllCaches() still clears only caches; disposeStylesheet() removes the element and drops the reference, and ChatPresentationService.cleanup() calls both. tests/assetTeardownRace.test.ts asserts the element is gone after teardown and that the next paint render rebuilds it.
The route created the synchronizer with a dispose() that cancels its pending navigation timer, but only the reduced-motion watch and the document lock were released. The timer could still fire after the setup route was left and assign src on the detached iframe.
The manager contract allows apply after cleanup; the stylesheet, the published properties and the boolean attributes all have to come back.
Both comments pointed at a Todo 5 that does not exist anywhere in the repository; they now describe the prop instead.
The follow-up is complete, so the documents now describe the tree that exists and no longer mix finished work with open work. - ARCHITECTURE.md gains the rule that a feature must not import a component, a lifecycle-and-asset-ownership section (construct/destroy contract, generation-guarded loads, the single-owner proxy argument), the layout and fade relocation, the new setup modules, and a plain/generated CSS section that matches the code. - DESIGN.md stops describing chatStyles.ts as the last remaining exception, and records the styling stack, iconography and surface ownership. - REFACTOR_PLAN.md closes every phase: phase 6 step 2 is closed without code movement with the reason, phase 6 step 3 is resolved by the service-layer move, phase 2's outcome no longer reads as open, and the stale statement about Hugeicons having no official Solid package is struck through with the correction. - REFACTOR_AUDIT.md gains a Post-refactor state section that keeps the original scores, records the after-score and the final owner per issue, and lists the two problems the audit did not predict. - REFACTORING_FOLLOWUP.md gains the completion report required by its section 47, including the items intentionally left unchanged and the measured verification results.
A review pass over the follow-up found two owners that released less than they started. LayoutManager.cleanup() removed the stylesheet but left a smooth follow's animation frame scheduled; it now cancels the frame and drops the follow state. PreviewRuntime.initialize() reset its destroyed flag, so a destroyed runtime could be re-initialized and re-claim the proxy flag and the shared stores; destruction is terminal there too, which is what the application contract already assumed. clearAllCaches() keeps the paint catalogue and the user-to-paint assignments on purpose, so its comment now says what it clears and why, instead of claiming every channel- and user-bound cache. Both new behaviours have tests: cleanup cancels the frame it started and removes the stylesheet, and a destroyed preview runtime cannot be initialized again.
Adds the two teardown fixes, names the four third-party badge services and the retained 7TV paint data among the items intentionally left unchanged, and refreshes the test count.
Collaborator
Author
|
Superseded by the follow-up PR. The branch gained 15 commits completing \documents/REFACTORING_FOLLOWUP.md\ (lifecycle ownership, dependency direction, the official Hugeicons Solid renderer, docs reconciliation) and this PR is replaced by a new one that describes the whole branch, including the 28 commits reviewed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Executes
documents/REFACTOR_PLAN.md— the phased plan derived fromdocuments/REFACTOR_AUDIT.mdand the rules indocuments/REFACTORING.md.28 commits, one concern each. The tree is green at every commit.
Review guide
Read top to bottom; each commit builds on the previous state. The last column is
how the change was verified, so you can judge whether the evidence matches the
risk.
92321fcdocs: add refactoring guide, audit and phased plan1eba59afix(chat): keep badge visibility reactiveChatBadgesreturned beforecreateMemo, sohideAllBadgeswas mount-time only4484e2frefactor(icons): consolidate ui icons onto a typed packagedist−1.35 MBa628860docs: record phase 3 outcomee134f83refactor(styles): remove dead legacy tokensbodycolour and--borderresolution17967cechore(chat): remove superseded modulesfd31334docs: record phase 2 and 7 progress19f2d9drefactor(setup): extract chip input018174arefactor(chat): remove duplicated render helpersrenderMessageContent+messageTokenSnapshotsuites unchanged2e4af5brefactor(setup): extract persistence adapterf10af40refactor(setup): extract bot profile lookupbc618afrefactor(setup): extract local font capabilityempty; injected fake → 2 families, styles grouped and sorted1040b15refactor(setup): extract form value coerciontests/formValues.test.ts, including the form→URL→config round trip2779a89refactor(setup): extract overlay url projectiontests/setupConfig.test.ts; browser: channel/size/line-height change the link and survive reload; preview dropsms83b24b7refactor(setup): extract preview synchronizertests/previewSync.test.ts(burst collapses to one navigation, dispose, origin); browser: commit navigates, config-only change does not4a73f61refactor(setup): extract document lifecycle helpers01e5d8drefactor(setup): extract settings search1/8 → 2/8 → 3/8 → 2/8b8afa9erefactor(setup): extract user chip and ffz merge block01c094brefactor(chat): give singleton caches and generated styles a teardown ownertests/serviceReset.test.ts; MutationObserver trace across teardownb96c637refactor(chat): merge entry animation cleanup, colocate badge priorityflowandfade9cdb2a6refactor(styles): publish overlay presets as custom properties03f563frefactor(chat): split animation vocabulary from style generationflowandfadestill animate and clean up8c56ca3refactor(chat): inject url resolution into the message renderer8b096a2refactor(setup): extract section descriptorsc652d5crefactor(setup): extract preview controls with thumb measurementtranslateY(40px)/0, height36px), pause toggles9291b8achore(build): drop the removed icon package from the ui chunk groupf7e1ec7fix(chat): release the animation stylesheet with the overlay stylestests/overlayStyleManager.test.ts; browser lifecycle trace1c8965ddocs: record the closing verificationCommits worth the most scrutiny: 14 (overlay URL is a contract with deployed
OBS sources), 21 (the whole overlay's computed styling), 19 and 27 (lifecycle),
10 (persistence).
What changed, by area
Icons.
@hugeicons/core-free-iconsplus a typedsrc/components/ui/icon.tsxwrapper. No icon font, no
hgi-*class string, no inline SVG standing in for a UIicon,
lucide-solidremoved. Icons import by subpath because the package barrelis 675 KB while one icon is under a kilobyte. Monochrome platform glyphs have one
owner in
src/components/brand/PlatformGlyph.tsx.Design tokens.
src/styles/chatStyles.tsno longer generates rules whoseshape follows a preset.
getOverlayStyleVariablesreturns a property map the waygetChatEventStyleVariablesalready did;chat.cssowns every rule and theruntime writes only custom properties and two root attributes.
SIZE_CONFIGSstays as the single source of truth because
renderMessageContentreads it foremote geometry.
Setup decomposition.
src/routes/setup.tsxwent from 2779 to 1686 lines.It is now a UI composition root: persistence, bot lookups, font capability, value
coercion, the config/URL projection, preview synchronisation, document lifecycle
and settings search all live outside it.
Lifecycle.
liveRuntime.destroy()releases everythinginitialize()installed — the generated stylesheets, the shared RTE proxy flag, and the badge
and emote caches, which are keyed by channel and previously outlived the runtime
that filled them.
Bugs found and fixed along the way
These were not the stated goal of any phase; they surfaced because the
verification was behavioural rather than structural.
injectAnimationStylesappended unconditionally, so two#chat-animationselements existed and removing "the" element by id removed only one.
#chat-layoutwas never removed on teardown at all.chatPresentationService.start()could re-inject styles afterdestroy().ChatBadgesreadprops.config.hideAllBadgesbefore itscreateMemo, makingthe setting mount-time only.
style manager dropped the animation-stylesheet teardown, so
#chat-animationsoutlived the runtime. Fixed in
f7e1ec7with a regression test.Verification
Beyond the suite, each phase was checked against the running app in a browser
(Playwright, installed outside the repo so
package.jsonis untouched): setupround-trip, overlay rendering, preview synchronisation, animation cleanup,
settings search, teardown traces and the computed-style matrix.
The two highest-risk changes were verified by diffing measurements taken before
and after, not by reasoning:
only differences are inherited
--chat-gigantified-emote-widthreadings withnone on the element that consumes it, readings of an artificial
.emojiprobewhose real counterpart sets the same size inline, and one container height
inside the measured noise floor — two runs of the same code differ by a row
height there.
MutationObservertrace shows the running overlay holding onlythe animation and layout stylesheets (generated by design) and the teardown
removing both while releasing every published property.
Known outstanding
Two items are recorded in
documents/REFACTOR_PLAN.mdrather than done:renderMessageContent.tsinto tokenize/assemble/DOM-bind files.The part that mattered — it no longer imports a network client, so it renders
without infrastructure — is done. The seam already exists in the code as a
token array. The physical split is a wide mechanical change to a file that two
suites protect, and it buys organisation rather than testability.
features/chat-overlay. Blocked bylayering, not effort:
chatPresentationService(a service) owns it, and themove would make a service import a feature.
Component-render characterization tests (phase 1, items 2–4) remain deferred by
the earlier decision to keep
happy-domout of the project; those behaviours arecovered by browser checks instead.
Breaking changes
None. No query parameter changed, so deployed OBS browser sources keep
working.
tests/setupConfig.test.tsandtests/formValues.test.tspin theconfig round trip, and
buildOverlayUrlis still the single producer of theoverlay link.
Notes for the reviewer
documents/REFACTOR_PLAN.mdcarries a live status column and an outcome noteper step, including what was verified and how. It is the fastest way to see the
intent behind any commit.
!importantaudit is recorded in phase 7: every remaining use sits at aboundary section 13 allows, and none was added by this branch.