Skip to content

fix(core): resolve inspector text edits for mixed-children pass-throughs - #254

Open
yen0304 wants to merge 1 commit into
1weiho:mainfrom
yen0304:fix/inspector-mixed-children-passthrough
Open

fix(core): resolve inspector text edits for mixed-children pass-throughs#254
yen0304 wants to merge 1 commit into
1weiho:mainfrom
yen0304:fix/inspector-mixed-children-passthrough

Conversation

@yen0304

@yen0304 yen0304 commented Jun 20, 2026

Copy link
Copy Markdown

Fixes #212

Problem

Inspector inline text editing fails with element has no editable text whenever the clicked text maps to a host element whose children are mixed — i.e. a {children}/prop pass-through alongside other children, with no literal text of its own:

<div style={s.row}><span style={s.label}>{label}</span>{children}</div>

Root cause

collectElementTextCandidates() falls back to resolving a pass-through to its call sites via propPassthroughName(), which bails out unless the pass-through is the element’s sole meaningful child:

function propPassthroughName(element) {
  const meaningful = meaningfulChildren(element);
  if (meaningful.length !== 1) return null; // <- mixed children never resolve
  ...
}

So an element that mixes {children} (or a {prop}) with anything else matches none of the three fallbacks, and buildTextSplice() returns element has no editable text.

Fix

Collect pass-through names from the element’s direct children instead of requiring a single child. A {children} or {prop} expression container that sits next to other children is still resolved to the matching call site(s). This only adds resolutions that previously failed — the sole-child cases behave exactly as before.

Tests

Added two cases to edit-ops.test.ts, both of which fail on main (element has no editable text) and pass with this change:

  • a {children} slot mixed with a nested {label} span, edited with no prevText;
  • a {prop} pass-through mixed with a {children} slot, routed to the title="…" literal at the call site.

pnpm vitest run packages/core243 passing, and pnpm --filter @open-slide/core typecheck is clean.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed inspector text edits for components with mixed children patterns, where host elements combine {children} and/or prop pass-through with additional children.

collectElementTextCandidates only resolved a {children}/prop pass-through to its call
sites when it was the host element's sole meaningful child (propPassthroughName bailed
when meaningfulChildren(element).length !== 1). A host like
<div><span>{label}</span>{children}</div> therefore matched none of the fallbacks and the
inspector reported "element has no editable text".

Collect pass-through names from the element's direct children instead, so a {children}
or prop pass-through mixed with other children still resolves to the call site.
@vercel

vercel Bot commented Jun 20, 2026

Copy link
Copy Markdown

@yen0304 is attempting to deploy a commit to the open-slide Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 79a14d41-404b-46a2-a9a4-a6312eb91af4

📥 Commits

Reviewing files that changed from the base of the PR and between 16fdd01 and 38b6263.

📒 Files selected for processing (3)
  • .changeset/inspector-mixed-children-passthrough.md
  • packages/core/src/editing/edit-ops.test.ts
  • packages/core/src/editing/edit-ops.ts

Walkthrough

Replaces the single-passthrough heuristic (propPassthroughName) in collectElementTextCandidates with a new directPassthroughNames helper that collects all bare-identifier JSXExpressionContainer children. The updated function iterates over every passthrough name to gather text candidates, fixing the bug where mixed-children host elements produced no editable text candidates. Two regression tests and a patch changeset are added.

Changes

Inspector mixed-children passthrough fix

Layer / File(s) Summary
directPassthroughNames helper and collectElementTextCandidates update
packages/core/src/editing/edit-ops.ts
Adds directPassthroughNames which returns all bare-identifier pass-through names from direct JSXExpressionContainer children. Updates collectElementTextCandidates to iterate over all passthrough names (de-duplicated via a Set), resolve an enclosing component only when passthroughs exist, and dispatch each name to collectCallSiteCandidates or collectPropCallSiteCandidates.
Regression tests and changeset
packages/core/src/editing/edit-ops.test.ts, .changeset/inspector-mixed-children-passthrough.md
Two new applyEdit tests cover: (1) set-text routing through {children} when mixed with a labeled <span>, and (2) prop pass-through routing when {children} and {title} coexist in the same host element. Changeset marks this as a patch release.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 Hop hop through the JSX tree,
Where mixed children once stumped me,
A single passthrough? Nay, collect them all!
No more "no editable text" to befall.
Each {prop} and {children} found in line,
Now routes to call sites — works just fine! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main fix: resolving inspector text edits for elements with mixed children and pass-throughs, matching the core problem described in issue #212.
Linked Issues check ✅ Passed The PR fully addresses issue #212 by implementing the relaxed detection logic to handle mixed children, allowing pass-throughs alongside other children to resolve to their call sites.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the mixed-children pass-through detection issue: the changeset, test cases, and the core edit-ops.ts logic changes directly address issue #212.

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


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.

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.

[Bug]: Inspector "element has no editable text" for host elements with mixed JSX children

1 participant