Skip to content

fix: prevent onboarding hints after anchor removal - #236

Merged
vanilla-wave merged 3 commits into
mainfrom
fix-onboarding-hint-anchor-removal
Oct 2, 2026
Merged

vanilla-wave merged 3 commits into
mainfrom
fix-onboarding-hint-anchor-removal

Conversation

@kseniya57

Copy link
Copy Markdown
Contributor

Onboarding hints could appear after their anchor was removed while asynchronous checks or promo initialization were still pending, leaving a detached popup and an active promo.

Recheck anchor connectivity before displaying the hint and skip the promo if its anchor disappears during startup. This preserves onboarding progress and allows the hint to appear when the user returns.

@kseniya57 kseniya57 self-assigned this Oct 2, 2026
@kseniya57
kseniya57 requested a review from vanilla-wave October 2, 2026 06:47
@vanilla-wave

Copy link
Copy Markdown
Collaborator

One scenario this breaks: the anchor is remounted (not just removed) while requestStart is pending.

Sequence with init: {initType: 'timeout'}:

  1. Element A calls stepElementReached → beforeShowHint → requestStart queues the promo and awaits ensureInit.
  2. React remounts the component (re-render with a new key, StrictMode, list re-render): A is detached, element B calls stepElementReached → second requestStart awaits the same init.
  3. Init resolves. Call A runs triggerNextPromo → activatePromo, gets true, then sees A.isConnected === false → skipPromo (clears activePromo and removes the slug from the queue).
  4. Call B continues: queue is empty, activePromo === null → returns false → skipPromo again.

Result: the hint never shows for B even though B is in the DOM, and nothing re-triggers it (skipPromo doesn't call checkReachedHints). Before this change the hint was rendered against the detached A, which was also wrong, but now it silently disappears until the next mount.

Suggestion: check the current anchor instead of the one captured in the closure, e.g. in the promo hook:

const currentElement = instance.reachedElements.get(stepData.stepSlug);
if (!result || !currentElement?.isConnected) {

and in controller.ts after beforeShowHint, swap element for this.reachedElements.get(stepSlug) if that one is connected rather than returning. Alternatively call getInstance().checkReachedHints() after skipPromo so a live anchor re-requests the promo.

Would be good to add a test for "anchor replaced with a new one before init resolves → hint shows on the new element".

@vanilla-wave
vanilla-wave merged commit 9baaf8e into main Oct 2, 2026
4 checks passed
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