Skip to content

fix: remove auto-open of demo onboarding modal#1728

Open
ToriChanIntegration wants to merge 1 commit intoCap-go:mainfrom
ToriChanIntegration:fix/remove-auto-demo-modal
Open

fix: remove auto-open of demo onboarding modal#1728
ToriChanIntegration wants to merge 1 commit intoCap-go:mainfrom
ToriChanIntegration:fix/remove-auto-demo-modal

Conversation

@ToriChanIntegration
Copy link

@ToriChanIntegration ToriChanIntegration commented Mar 1, 2026

The demo modal no longer auto-opens when users have no apps. Users can still trigger it explicitly via ?show-onboarding-demo=1 query param.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Updated demo onboarding display to only appear when explicitly triggered via query parameter, removing unintended automatic displays based on app count and other conditions.

The demo modal no longer auto-opens when users have no apps.
Users can still trigger it explicitly via ?show-onboarding-demo=1 query param.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 1, 2026

📝 Walkthrough

Walkthrough

The DemoOnboardingGate.vue component's visibility logic was simplified. The shouldShowDemoOnNoApps condition now depends solely on an explicit query parameter (show-onboarding-demo=1) rather than evaluating multiple interdependent state properties and device/path conditions.

Changes

Cohort / File(s) Summary
Demo Onboarding Gate Logic
src/components/dashboard/DemoOnboardingGate.vue
Simplified visibility condition from multi-branch state evaluation to a single query parameter check. Removed dependencies on app count, onDemoOnboardingClosed, shouldShowDemoOnboarding, and isMobileView/path-based logic. Control flow now directly returns shouldForceShowDemoOnboarding.value.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Poem

🐰 A query parameter, clear and bright,
Replaces tangled logic's plight,
One condition now, so lean and lean,
The simplest gate we've ever seen!
No more branches, no more fuss,
Just show=1 for all of us! 🌟

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete relative to the template. It lacks required sections: Summary (only a one-liner provided), Test plan, Screenshots, and Checklist. Add missing template sections: expand Summary, include Test plan with reproduction steps, add Screenshots if UI behavior changed, and complete the Checklist.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: removing the auto-open behavior of the demo onboarding modal.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Contributor

@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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/dashboard/DemoOnboardingGate.vue`:
- Around line 57-58: The app-count fetch pipeline is running unnecessarily
because :open now depends only on shouldForceShowDemoOnboarding, making
fetchAppsCount/hasNoApps/shouldShowDemoOnboarding dead for render while still
executing; update the code so fetchAppsCount (and any watcher/effect that calls
it on init/org change) is short-circuited when
shouldForceShowDemoOnboarding.value is true — e.g., add a guard in the
initialization/watcher that checks shouldForceShowDemoOnboarding before invoking
fetchAppsCount or computing hasNoApps, or remove the unused reactive dependency
in shouldShowDemoOnboarding and relocate fetchAppsCount to only run when the
non-forced path is active, ensuring Supabase queries are skipped when forced
demo onboarding is enabled.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e1ed50a and df0b148.

📒 Files selected for processing (1)
  • src/components/dashboard/DemoOnboardingGate.vue

Comment on lines +57 to +58
// Only show when explicitly requested via query param
return shouldForceShowDemoOnboarding.value
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Line 58 makes the app-count fetch pipeline dead, but it still runs on every init/org change.

Since :open now depends on shouldShowDemoOnNoApps -> shouldForceShowDemoOnboarding, fetchAppsCount()/hasNoApps/shouldShowDemoOnboarding no longer affect rendering, yet Supabase apps count queries still execute. This adds avoidable backend load and noisy error paths.

♻️ Proposed cleanup (minimal, behavior-preserving)
 async function initDemoOnboarding() {
   await organizationStore.awaitInitialLoad()
-  updateDemoOnboardingState()
-  await fetchAppsCount()
 }
 
-watch(currentOrganization, async () => {
-  await initDemoOnboarding()
-})
+// No org-dependent app-count fetch needed for query-param-only gating.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/dashboard/DemoOnboardingGate.vue` around lines 57 - 58, The
app-count fetch pipeline is running unnecessarily because :open now depends only
on shouldForceShowDemoOnboarding, making
fetchAppsCount/hasNoApps/shouldShowDemoOnboarding dead for render while still
executing; update the code so fetchAppsCount (and any watcher/effect that calls
it on init/org change) is short-circuited when
shouldForceShowDemoOnboarding.value is true — e.g., add a guard in the
initialization/watcher that checks shouldForceShowDemoOnboarding before invoking
fetchAppsCount or computing hasNoApps, or remove the unused reactive dependency
in shouldShowDemoOnboarding and relocate fetchAppsCount to only run when the
non-forced path is active, ensuring Supabase queries are skipped when forced
demo onboarding is enabled.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 1, 2026

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