refactor(panels): framework selector gear button#2539
Conversation
…ear button The full <select> element in the panel header was visually heavy and inconsistent across 4 panels (DailyMarketBrief, MarketImplications, Deduction, Insights). Replaced with a compact gear icon-btn that opens a fixed-position popup containing the framework select and note text. Popup positions below the button, closes on outside click or selection, and uses aria-expanded for active state styling. Free-tier button redirects to the gated CTA as before.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Greptile SummaryThis PR refactors the analysis framework selector across all four AI panels (Daily Market Brief, Market Implications, Deduction, Insights) from an always-visible Key implementation details:
Minor findings (all P2):
Confidence Score: 5/5Safe to merge — all findings are P2 style/accessibility suggestions with no impact on correctness or runtime behaviour. The refactor is well-scoped: DOM lifecycle (detach/re-attach of the shared select node, event listener cleanup) is handled correctly, destroy() properly tears down the popup, and the CSS changes use existing design tokens. The three P2 findings are non-blocking quality improvements. src/components/FrameworkSelector.ts — minor aria and dead-code issues noted above, but nothing blocking. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant GearBtn as ⚙ Button
participant FS as FrameworkSelector
participant Body as document.body
participant Store as analysis-framework-store
User->>GearBtn: click
GearBtn->>FS: btn click handler (stopPropagation)
alt popup closed
FS->>FS: openPopup()
FS->>Body: appendChild(popup + select)
FS->>GearBtn: aria-expanded = "true"
FS->>Body: addEventListener("click", outsideClickHandler, setTimeout 0)
else popup open
FS->>FS: closePopup()
FS->>FS: popup.removeChild(select) — preserve select node
FS->>Body: popup.remove()
FS->>GearBtn: aria-expanded = "false"
FS->>Body: removeEventListener("click", outsideClickHandler)
end
User->>GearBtn: click outside popup
Body->>FS: outsideClickHandler fires
FS->>FS: closePopup()
User->>GearBtn: select option in popup
GearBtn->>Store: setActiveFrameworkForPanel()
GearBtn->>FS: updateBtnTitle()
GearBtn->>FS: closePopup()
Note over FS,Body: destroy() → closePopup() cleans up popup + listener
Reviews (1): Last reviewed commit: "refactor(panels): replace framework sele..." | Re-trigger Greptile |
| const btn = document.createElement('button'); | ||
| btn.className = 'icon-btn framework-settings-btn'; | ||
| btn.innerHTML = '⚙'; | ||
| this.btn = btn; |
There was a problem hiding this comment.
The button's aria-expanded attribute is never set in the constructor. Before the popup is opened for the first time, the button carries no aria-expanded attribute at all. Screen readers may not announce the collapsed state correctly, and any CSS that targets [aria-expanded="false"] will silently not match.
| const btn = document.createElement('button'); | |
| btn.className = 'icon-btn framework-settings-btn'; | |
| btn.innerHTML = '⚙'; | |
| this.btn = btn; | |
| btn.className = 'icon-btn framework-settings-btn'; | |
| btn.innerHTML = '⚙'; | |
| btn.setAttribute('aria-expanded', 'false'); | |
| this.btn = btn; |
| const handler = (e: MouseEvent) => { | ||
| if (!popup.contains(e.target as Node) && e.target !== this.btn) { | ||
| this.closePopup(); | ||
| } |
There was a problem hiding this comment.
e.target !== this.btn check is dead code
Both button click listeners (premium and free-tier branches) call e.stopPropagation(), so a click on this.btn is never propagated to the document-level handler. The condition e.target !== this.btn can therefore never be false, making it unreachable dead code.
The guard is harmless today but could mislead a future developer into thinking button-clicks reach this handler. It can be simplified to just:
| const handler = (e: MouseEvent) => { | |
| if (!popup.contains(e.target as Node) && e.target !== this.btn) { | |
| this.closePopup(); | |
| } | |
| const handler = (e: MouseEvent) => { | |
| if (!popup.contains(e.target as Node)) { | |
| this.closePopup(); | |
| } | |
| }; |
| private openPopup(): void { | ||
| const btnRect = this.btn.getBoundingClientRect(); | ||
|
|
||
| const popup = document.createElement('div'); | ||
| popup.className = 'framework-settings-popup'; | ||
| popup.style.top = `${btnRect.bottom + 4}px`; | ||
| popup.style.right = `${document.documentElement.clientWidth - btnRect.right}px`; |
There was a problem hiding this comment.
Popup position drifts on scroll / resize
top and right are computed once from getBoundingClientRect() at the moment the popup opens, then written as static position: fixed values. If the user scrolls the page (or resizes the window) while the popup is open, the gear button moves but the popup stays at the original coordinates, creating a visual disconnect.
Consider adding scroll and resize listeners (passive, on window) that call closePopup(), keeping behaviour consistent with other lightweight dropdowns in the app:
const closeOnScroll = () => this.closePopup();
window.addEventListener('scroll', closeOnScroll, { passive: true, capture: true });
window.addEventListener('resize', closeOnScroll, { passive: true });
// store refs and remove in closePopup()
Why
The analysis framework
<select>sitting directly in the panel header was visually noisy and inconsistent across all 4 panels that carry it (Daily Market Brief, Market Implications, Deduction, Insights).What changed
FrameworkSelector.ts: select element removed from the header DOM. Replaced by a small⚙icon-btn. Clicking it opens a compactposition: fixedpopup (appended tobody) containing the framework select + note text, positioned directly below the button. Closes on outside click or after making a selection.aria-expandedtoggles for CSS active state.main.css: Old.framework-selector*rules replaced with.framework-settings-btn,.framework-settings-popup,.framework-popup-select,.framework-settings-note.this.header.appendChild(this.fwSelector.el)call is unchanged in all 4 panels).Free-tier behavior is unchanged: the gear button opens the PRO gated CTA.