Skip to content

fix: allow users to exit out of create instance#2512

Open
finnar-bin wants to merge 4 commits into
stagefrom
fix/2511-add-way-to-exit-create-instance-flow
Open

fix: allow users to exit out of create instance#2512
finnar-bin wants to merge 4 commits into
stagefrom
fix/2511-add-way-to-exit-create-instance-flow

Conversation

@finnar-bin

Copy link
Copy Markdown
Contributor

Description

Allow users to exit out of the create instance flow

Fixes #2511

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Screenshots / Screen recording

Screencast_20251010_074001.webm

@finnar-bin finnar-bin self-assigned this Oct 9, 2025
@finnar-bin finnar-bin changed the title fix: allow users toexit out of create instance fix: allow users to exit out of create instance Oct 9, 2025
agalin920
agalin920 previously approved these changes Nov 17, 2025
Comment thread src/pages/start.js Outdated
@github-actions

Copy link
Copy Markdown

Review summary

Scope is small (one file, ~10 lines) and the goal is right, but the new control flow has a bug.

Blocker — router.back() runs on every back-click, not just when exiting the flow.
In src/pages/start.js handlePrev, after the if (currentStep > 1) { ... } block, router.back() is unconditionally executed. So on steps 2+, clicking back rewinds the swiper and immediately navigates the user off the start page. Inline suggestion posted with the fix (early-return after the slider rewind, or wrap router.back() in an else).

Minor

  • router should be in the useCallback dependency array.
  • Consider whether router.back() is the right exit when the user landed on /start directly (no history) — router.push('/') may be a safer default, but that's a UX call.

Otherwise nothing concerning from a security or performance perspective — no new data flows, no new network calls, just navigation logic.

Comment thread src/pages/start.js
@github-actions

Copy link
Copy Markdown

Review summary

Small, well-scoped fix that addresses #2511 — appreciated the focus. The change correctly restructures handlePrev so that step-1 falls through to a router action, and the early return after slidePrev() keeps the two branches cleanly separated.

Main concern (inline): router.back() is a no-op when there's no prior session history (direct navigation, fresh tab, refresh), which would leave the user stuck on step 1 — the exact scenario this PR is trying to unblock. Suggested a fallback to / (or another known route) inline.

Minor observations (non-blocking):

  • The existing handleNext / handleAnswers flow calls setCurrentStep(currentStep + 1) after handleNext()handlePrev here correctly mirrors that ordering in reverse, so behavior is consistent.
  • No new dependencies, no security surface added, and useRouter is correctly listed in the useCallback deps.

Nothing else blocking from a code-quality, security, or performance standpoint.

Comment thread src/pages/start.js
sliderRef.current.swiper.slidePrev();
}, [currentStep]);

router.back();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

router.back() is a no-op when there's no in-app history — e.g., if the user lands on /start directly via a shared link, email/marketing campaign, or new tab. In that case clicking the back chevron does nothing and the user is stuck on step 1 with no way to "exit".

Consider falling back to a known route when there's no history:

if (window.history.length > 1) {
  router.back();
} else {
  router.push('/');
}

Same caveat applies if the previous entry was external (e.g., Google) — router.back() will navigate the user off the site entirely, which may or may not be the intended "exit" behavior.

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown

Reviewed the change — small, focused, and matches the PR's goal of letting users exit the create-instance flow from step 1.

Code quality: Implementation is clean. The early-return pattern reads well, the useRouter import is correct, and the useCallback dependency array is updated appropriately. router.back() is already used elsewhere in the codebase (src/components/accounts/support/Chat.js), so this is consistent.

Bugs / UX: Flagged one edge case inline — router.back() is a no-op on direct navigation to /start (shared link, marketing campaign, new tab), which leaves the user stuck on step 1 with no exit. A fallback to a known route handles this. Also note that when the previous history entry is external, the back chevron will navigate users off the site — possibly fine for "exit," but worth a deliberate decision since the icon visually reads as "previous step."

Security: No concerns. No new inputs, no auth changes, no external calls.

Performance: Negligible — adds useRouter and one extra branch in an already-memoized callback.

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.

[Dashboard] - When a user click "Create Instance" there is no way to exit instance creation

3 participants