Skip to content

refactor(ui): extract shared UI factories#33

Merged
fathiraz merged 2 commits into
mainfrom
refactor/extract-ui-factories
May 28, 2026
Merged

refactor(ui): extract shared UI factories#33
fathiraz merged 2 commits into
mainfrom
refactor/extract-ui-factories

Conversation

@fathiraz

@fathiraz fathiraz commented May 28, 2026

Copy link
Copy Markdown
Owner

Summary

Extract duplicated UI boilerplate into three shared factories: createFeatureUi for shadow DOM content scripts, createModal for confirmation modals, and primerCss for SX style presets.

Changes

  • Add lib/shadow-ui-factory.tsxcreateFeatureUi() wraps WXT createShadowRootUi with StyleSheetManager, ShadowThemeProvider, ErrorBoundary, and optional Primer portal compat
  • Add lib/primer-css-helper.tsprimerCss namespace with 10 SX preset functions (buttonMotion, flatPanel, modalOverlay, etc.)
  • Add lib/modal-factory.tsxcreateModal<T>() HOC with overlay, escape key, loading/validation/error states, and toast notifications
  • Refactor content-ui.tsx from 259 to 120 lines — 5 factory calls replace 5 copy-pasted shadow root blocks
  • Migrate bulk modals (pin/open/unpin) from ~75 lines each to ~20 lines using createModal
  • Replace inline SX objects and button style functions with primerCss presets

Test Plan

  • pnpm typecheck passes with 0 errors
  • pnpm build:chrome, pnpm build:firefox, pnpm build:edge all succeed

Summary by cubic

Extracted shared UI factories to remove duplicated shadow-DOM and modal code, standardize styles, and simplify maintenance. This reduces lines across content UIs and modals, ensures consistent theming/overlays, and hardens cleanup and modal behavior.

  • New Features

    • createFeatureUi: wraps wxt shadow UI with styled-components StyleSheetManager, ShadowThemeProvider, ErrorBoundary, optional Primer portal compat, and robust destroy() cleanup that calls WXT remove().
    • createModal<T>: modal factory with overlay, Escape-to-close, validation/loading/error states, success toasts, and protections that block close during submit and guard async state updates.
    • primerCss: SX style presets (e.g., buttonMotion, modalOverlay, modalPanel, contentArea) for consistent UI with @primer/react.
  • Refactors

    • Replaced five shadow-root blocks with createFeatureUi in content-ui.tsx (259 → 120 lines) and centralized Primer portal setup.
    • Migrated bulk modals (open/pin/unpin) to createModal (~75 → ~20 lines each) with built-in handling.
    • Swapped inline SX/button helpers for primerCss presets across features and ui/actions.tsx.

Written for commit 0e2cadf. Summary will update on new commits.

Review in cubic

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor auto review

Found 3 actionable issue(s) on changed lines.

Refactor looks directionally good, but there are a couple of high-confidence runtime issues in the new factories (missing import and incomplete teardown), plus a potential setState-on-unmounted path in the modal factory when closing during async submit.

Generated automatically when this PR was submitted using Cursor CLI with --model auto.

let root: ReactDOM.Root | null = null
let styleHost: HTMLDivElement | null = null

const ui = await createShadowRootUi(ctx, {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createShadowRootUi is used here but not imported in this file, which will fail at build/runtime. Import it from WXT (same place it was previously imported from in content-ui.tsx).

mount() {
ui.mount()
},
destroy() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

destroy() currently unmounts React and removes styleHost, but it never tells the underlying WXT UI to unmount/remove its container/shadow root. This can leave DOM/UI artifacts around on invalidation; call the WXT UI cleanup method as part of destroy() (and consider also nulling refs).

Comment thread src/lib/modal-factory.tsx Outdated

useEffect(() => {
const handleKeyDown = (e: KeyboardEvent) => {
if (e.key === 'Escape') onClose()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This closes the modal on Escape even while loading is true. If the async onSubmit later rejects, the catch path will call setError/setLoading on an unmounted component. Consider disabling close (Escape/overlay click) while loading and/or guarding state updates with an isMounted ref.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 9 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/lib/shadow-ui-factory.tsx Outdated
Comment thread src/lib/modal-factory.tsx
- Import createShadowRootUi in shadow UI factory
- Ensure destroy() calls WXT remove() cleanup
- Prevent modal close during submit + guard async state updates
@fathiraz

Copy link
Copy Markdown
Owner Author

re-review @cubic-dev-ai

@cubic-dev-ai

cubic-dev-ai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

re-review @cubic-dev-ai

@fathiraz I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 9 files

Re-trigger cubic

@fathiraz fathiraz merged commit 9ffd5b1 into main May 28, 2026
3 checks passed
@fathiraz fathiraz deleted the refactor/extract-ui-factories branch May 28, 2026 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant