Skip to content

fix: Progress Bar Unification#593

Closed
MaanilVerma wants to merge 90 commits into
Comfy-Org:mainfrom
MaanilVerma:fxi/progress-bar-unification
Closed

fix: Progress Bar Unification#593
MaanilVerma wants to merge 90 commits into
Comfy-Org:mainfrom
MaanilVerma:fxi/progress-bar-unification

Conversation

@MaanilVerma

Copy link
Copy Markdown
Collaborator

Summary

Unifies the brand progress takeover so install→launch chains read as one continuous 0→100% journey instead of the install bar saturating at 100% and stalling through launch. Adds chainSpan plumbing from useFirstUseChain through the progress store into ProgressModal, maps install to 0–70% and launch to 70–100%, and replaces the rolling launch caption with a five-step stepper plus polished substatus formatting.

Changes

General Status:

  • No functional regressions + all test cases pass.

Progress Bar Unification:

  • Added chainSpan on ShowProgressOpts / Operation ('install' | 'launch' | null) so chained ops can share one visual progress arc.
  • Install leg (chainSpan: 'install') maps the existing stepped/global progress to 0→70% — the bar never visually completes before launch begins.
  • Launch leg (chainSpan: 'launch') maps launch progress to 70→100%, picking up where the install left off.
  • Standalone ops (no chainSpan) keep their existing 0→100% behaviour unchanged.

First-Use / Auto-Launch Chain:

  • useFirstUseChain stamps chainSpan: 'install' on:
    • Express first-use install ops
    • Any op with autoLaunchOnFinish: true
  • When the auto-launch watcher fires performChooserLaunch, the subsequent launch op is stamped chainSpan: 'launch'.
  • pendingChainedLaunch is cleared in .finally() so a launch that never reaches show-progress (missing-action / focused-running) cannot leak the flag into unrelated ops.
  • usePanelOverlays forwards chainSpan into progressStore.startOperation.

ProgressModal UX:

  • unifiedPercent — single bar math across install, chained launch, and standalone launch.
  • Launch stepper — five rows (security scan → mount libraries → GPU → custom nodes → starting server) with done / active / pending states and spinner on the active row; replaces the old rolling caption for brand launch ops.
  • launchPercent — slot-weighted bar interpolation within each launch step so the bar moves between step transitions even when advancement is timer/stdout-driven.
  • Substatus polish — locale-groups large byte counts (2100 MB2,100 MB), trims collapsed 0s remaining tails.
  • Launch ops show the stepper + unified bar; non-launch ops keep the determinate bar + percent label (indeterminate animation suppressed for chained install legs).

Tests:

  • progressStorechainSpan persistence and no regression in globalProgressFor.
  • useFirstUseChain — install/launch stamping, flag cleanup on missing-action outcomes.
  • ProgressModal — unified bar mapping, launch stepper states, substatus formatting, updated success/cancel assertions for the redesigned footer.

Review Focus

  • 70 / 30 split — Install caps at 70%, launch starts at 70%. Worth a feel-check on express install and auto-launch-on-finish flows: does the seam between the two takeover legs feel continuous, or does the bar jump/stall at the handoff?
  • pendingChainedLaunch lifecycle — Flag is set before performChooserLaunch and consumed in onShowProgress; .finally() clears it if launch never emits show-progress. Confirm no mis-stamping on a second unrelated op after a failed/missing launch.
  • Install bar vs launch stepper — Chained install still renders the horizontal bar (0–70%); chained launch swaps to the five-row stepper (70–100%). Verify the UX reads as one journey, not two disconnected surfaces.
  • launchPercent interpolation — Bar motion inside a step is time-budgeted (LAUNCH_STEP_BUDGET_MS), while step advancement is still timer + stdout-driven. Quick check that the bar never outruns the active step label.
  • globalProgressFor unchangedchainSpan remapping lives in the view only; stepped install ops still report raw weighted percent from the store. Confirm no double-scaling if main-side phase weights shift.
  • Standalone ops — Delete / update / snapshot / generic ops with no chainSpan should behave exactly as before (including indeterminate phases).
  • Substatus formatter — Byte-count grouping is scoped to unit suffixes (MB, GB, etc.); sanity-check rich download strings don't mangle ports, PIDs, or ratios.

Centre the layered beam against the responsive chooser spotlight and
align BrandBackground defaults formatting with Vue style.
Add frosted backdrop on the shared shell so layered modals read against
host UI instead of a flat wash.
Drop bespoke overlay wiring and reuse the shared close + focus +
backdrop behaviour; update the test to target the primitive close.
Align the explainer backdrop with BaseModal frosted stacking.
Tone down checkbox hit target on brand consent rows so they match the
density of merged start-step layout.
Support first-use merged screen: radio chrome, accent selection state,
optional label trailing slot when the arrow is hidden.
Update consent copy, telemetry line, express-install labels for en/zh,
and keys used by Continue on the combined start sheet.
Only start and localBranch are valid reopen targets after consolidating
consent + fork screens.
Mirror the narrower initialStep union when routing back into takeover.
Single Continue persists telemetry, records fork telemetry, honours
mirror + skip gates, routes cloud/local/legacy flows, and uses radio
cards with tooltip affordances.
Exercise Continue gate, stubs ChoiceCard tooltip slot + TooltipWrap mock.
Keep lockdown takeover minimal alongside other hidden toolbar actions.
Support bubble ref collision detection alongside the existing Top/Bottom
offset API used by TooltipWrap-era callers.
Teleport bubble, viewport flip/clamp + arrow tracing; tests stub rects to
drive placement edges.
Preserve legacy import paths while migrating behaviour to the shared
primitive (side + align forwarded).
Defer placement and bubble chrome to `/ui`; add focus-visible icon lift
and side/delay forwarding.
Avoid an empty ProgressModal flash by starting show-progress IPC before
Tier-3 takeover swap then wiring ProgressModal with showOperation only.
Skip Configure using recommended standalone field picks when chain-local
receives express, with graceful fallback plus coverage for the IPC path.
Add startContinueBusy for the spinner phase during express preparation.
Use Tooltip primitive for cloud hint, pass express payload on chain-
local when legacy express bypass applies, spinner + aria-busy on multi-
IPC continuation.
Exercise chain-local payloads, precedence over localBranch when express
defaults on, and Continue busy label during await.
Expose --tooltip-bg/border/fg and --z-tooltip for the Tooltip primitive
in dark and branded light palettes.
Throttle placement through rAF, attach viewport reflow listeners while
open, and remove the obsolete legacy overload — only callers use bubble
measurement paths now.
Wire aria-describedby to the bubbled tip, Escape to dismiss, semantic
bubble id via useId, and route chrome through tooltip CSS variables.
Default overlays stay translucent-only; callers opt into blur/saturation
via blurOverlay where the design warrants the GPU hit.
Tune blur/saturation down to pair with optional BaseModal overlay blur.
Expose an open prop so focus restore runs on close from FirstUse gates,
and enable the frosted backdrop for legal-reading surfaces.
Only the selected ChoiceCard participates in sequential focus so arrow
routing can steer between radio options cleanly.
Treat tier-3 takeovers as dialogs with modal semantics, restore focus on
leave, and auto-focus first interactive affordance post-mount.
Let Cloud/Local cards follow arrow-key browsing pattern, document skipPick
intent, bind TermsModal open to termsDoc for correct focus teardown.
Fill the picker panel with modal-surface-bg after main merge; update panel
test to match expanded-mode removal from open-settings deep link.
@Kosinkadink

Copy link
Copy Markdown
Member

superceded by another pr

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.

2 participants