fix: variable popover no longer opens on range selection in email editor

[tejassinghbhati]

Closes #277

Problem

Selecting text in the email editor that contains variable chips popped open the fallback-value input on every variable in the selection, making it impossible to cleanly select and copy content in or out of the editor (see the screen recording in the issue).

Root cause: the popover was bound directly to tiptap's selected NodeView prop (<Popover open={props.selected}>). tiptap sets selected whenever the node is covered by any selection range, including drag and keyboard range selections, not just direct clicks on the node.

Fix

Track popover visibility in component state driven by the editor's selectionUpdate event, and open it only when the current selection is a NodeSelection whose position matches this node, i.e. the user actually clicked the variable chip.

• Range selections over variables now behave like plain text selection: no popovers, copy works.
• Clicking a chip still opens the fallback input exactly as before.
• The chip keeps its ProseMirror-selectednode highlight when inside a range selection (still uses props.selected), so visual selection feedback is unchanged.
• Listening to selectionUpdate (rather than a render-time check) also covers the edge where tiptap skips re-rendering because selected didn't change, e.g. extending a node selection into a range selection with shift+arrows.

Changes

• packages/email-editor/src/nodes/variable.tsx: popover state from selectionUpdate + NodeSelection check; removed a stray console.log

Verification

• tsc --noEmit on packages/email-editor passes
• Manual: in the template editor, drag-select across text containing {{variables}}, no popovers appear and copy/paste works; click a chip and the fallback popover opens as before

Summary by CodeRabbit

Summary

• Bug Fixes
  • Updated the variable fallback popover to open only when the variable node itself is directly selected.
  • Prevented the popover from appearing for unrelated selections or cursor positions.
  • Removed unintended console output related to variable selection.

[vercel]

@tejassinghbhati is attempting to deploy a commit to the kmkoushik's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

Walkthrough

Updated the variable node popover to open only when the editor has a NodeSelection directly targeting that variable. Added selection update state management with listener cleanup, replaced the props.selected open condition, and removed a debug log.

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The change matches #277 by stopping popovers on range selection while preserving direct variable chip interaction.
Out of Scope Changes check	✅ Passed	The edits stay scoped to the variable popover behavior in the email editor, with only a minor console log removal.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main fix: variable popovers no longer open during range selection in the email editor.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/email-editor/src/nodes/variable.tsx`:
- Line 210: Add an onOpenChange handler to the controlled Popover in the
variable node so Radix close events update the same state used by popoverOpen.
Ensure Escape and clicks outside the editor set popoverOpen to false while
preserving the existing selectionUpdate behavior for editor-internal clicks.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: aea17f52-cb96-412c-b587-e86f49f28117

📥 Commits

Reviewing files that changed from the base of the PR and between 28970c9 and e21d5c4.

📒 Files selected for processing (1)

• packages/email-editor/src/nodes/variable.tsx

[coderabbitai]

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Add onOpenChange to handle Escape and click-outside-editor close.

The Popover is now controlled by popoverOpen but lacks an onOpenChange handler. Radix fires onOpenChange(false) on Escape and external clicks; without a handler, the popover stays open in those cases. The selectionUpdate listener covers clicks inside the editor, but Escape and clicks outside the editor are unhandled.

🔧 Suggested fix

-      <Popover open={popoverOpen}>
+      <Popover open={popoverOpen} onOpenChange={setPopoverOpen}>

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	<Popover open={popoverOpen}>
	<Popover open={popoverOpen} onOpenChange={setPopoverOpen}>

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/email-editor/src/nodes/variable.tsx` at line 210, Add an
onOpenChange handler to the controlled Popover in the variable node so Radix
close events update the same state used by popoverOpen. Ensure Escape and clicks
outside the editor set popoverOpen to false while preserving the existing
selectionUpdate behavior for editor-internal clicks.