Skip to content
This repository was archived by the owner on May 1, 2026. It is now read-only.

fix: eliminate flash frame between step transitions#567

Merged
WcaleNieWolny merged 1 commit into
mainfrom
fix/eliminate-step-transition-flash
Mar 23, 2026
Merged

fix: eliminate flash frame between step transitions#567
WcaleNieWolny merged 1 commit into
mainfrom
fix/eliminate-step-transition-flash

Conversation

@WcaleNieWolny
Copy link
Copy Markdown
Contributor

@WcaleNieWolny WcaleNieWolny commented Mar 23, 2026

Summary

  • Remove s.stop() from addAppStep success path — spinner persists through the async markStep call instead of leaving an empty frame that Ink renders
  • Add setInitSpinner() to renderInitOnboardingFrame so any running spinner is cleared atomically with clearInitLogs() + setInitScreen() on step transition
  • Move "accessible to all members" info to start of step 2 (addChannelStep) where it persists on screen

Test plan

  • Run node dist/index.js i through app add — no flash between step 1 completing and step 2 appearing
  • Step 2 shows ✅ App <id> added — accessible to all members of your organization persistently
  • Error path still works: spinner stops with "App add failed ❌" on failure

Summary by CodeRabbit

  • Refactor
    • Reorganized success message timing during app initialization and channel creation workflows
    • Improved spinner initialization in the onboarding UI for better visual feedback sequencing

- Remove s.stop() from addAppStep success path so the spinner persists
  through the async markStep call instead of leaving an empty frame
- Add setInitSpinner() to renderInitOnboardingFrame so any running
  spinner is cleared atomically with the step transition
- Move "accessible to all members" info to step 2 where it persists
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 23, 2026

📝 Walkthrough

Walkthrough

This PR reorganizes the onboarding spinner and logging behavior by centralizing spinner state management into renderInitOnboardingFrame(), removing the spinner completion message and info log from addAppStep, and relocating the success log to addChannelStep.

Changes

Cohort / File(s) Summary
Command Step Refactoring
src/init/command.ts
Removed spinner completion message and follow-up info log from addAppStep; added success log indicating app accessibility to addChannelStep.
UI Spinner Integration
src/init/ui.ts
Imported setInitSpinner from ./runtime and integrated setInitSpinner() call in renderInitOnboardingFrame() after clearing logs and before step data retrieval.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 Spinners spin where logs once chimed,
Success moves onward, well-aligned,
From step to frame, the flow takes flight,
Onboarding glows with clearer light!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: eliminate flash frame between step transitions' directly and clearly summarizes the main change—eliminating visual flashing between step transitions by managing spinner persistence and log clearing atomically.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/eliminate-step-transition-flash

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/init/command.ts (1)

1075-1086: ⚠️ Potential issue | 🟠 Major

Stop the spinner when markStep fails after a successful app add.

If app creation succeeds but markStep(...) throws, the spinner remains active while the error propagates, leaving stale spinner UI on failure exit.

Suggested fix
       try {
         await addAppInternal(currentAppId, options, organization, true)
       }
       catch (innerError) {
         s.stop(`App add failed ❌`)
         throw innerError
       }

-      await markStep(organization.gid, apikey, 'add-app', currentAppId)
-      return currentAppId
+      try {
+        await markStep(organization.gid, apikey, 'add-app', currentAppId)
+        return currentAppId
+      }
+      catch (markError) {
+        s.stop('Could not save onboarding progress ❌')
+        throw markError
+      }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/init/command.ts` around lines 1075 - 1086, The spinner started via
pSpinner() (variable s) is stopped only on addAppInternal errors; if
markStep(organization.gid, apikey, 'add-app', currentAppId) throws after a
successful add, the spinner remains active — ensure s.stop() is always called by
wrapping the markStep call (and the return) in a try/catch/finally or by adding
a try around markStep that calls s.stop() in both success and error paths;
reference pSpinner(), s, addAppInternal, markStep, organization.gid, apikey, and
currentAppId when making the change so the spinner is stopped before propagating
any error or returning the currentAppId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/init/command.ts`:
- Around line 1075-1086: The spinner started via pSpinner() (variable s) is
stopped only on addAppInternal errors; if markStep(organization.gid, apikey,
'add-app', currentAppId) throws after a successful add, the spinner remains
active — ensure s.stop() is always called by wrapping the markStep call (and the
return) in a try/catch/finally or by adding a try around markStep that calls
s.stop() in both success and error paths; reference pSpinner(), s,
addAppInternal, markStep, organization.gid, apikey, and currentAppId when making
the change so the spinner is stopped before propagating any error or returning
the currentAppId.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7df5c782-a7c4-44b5-bb74-caf0e21f4cf1

📥 Commits

Reviewing files that changed from the base of the PR and between 776895f and 43e1b78.

📒 Files selected for processing (2)
  • src/init/command.ts
  • src/init/ui.ts

@WcaleNieWolny WcaleNieWolny merged commit 24e060b into main Mar 23, 2026
19 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant