Skip to content

refactor: code fieldType#1151

Merged
mkilpatrick merged 43 commits intomainfrom
code-field
Apr 10, 2026
Merged

refactor: code fieldType#1151
mkilpatrick merged 43 commits intomainfrom
code-field

Conversation

@mkilpatrick
Copy link
Copy Markdown
Collaborator

@mkilpatrick mkilpatrick commented Apr 8, 2026

Makes code a real fieldType.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 8, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 275ed5a0-98fe-4b06-b20c-de5471e334c3

📥 Commits

Reviewing files that changed from the base of the PR and between 9e255e2 and 7dec0de.

📒 Files selected for processing (4)
  • packages/visual-editor/src/editor/FontSizeSelector.tsx
  • packages/visual-editor/src/editor/YextField.test.tsx
  • packages/visual-editor/src/fields/BasicSelectorField.tsx
  • packages/visual-editor/src/internal/components/AdvancedSettings.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/visual-editor/src/internal/components/AdvancedSettings.tsx
  • packages/visual-editor/src/editor/YextField.test.tsx

Walkthrough

This PR removes the editor-level CodeField and reintroduces it as a field override at packages/visual-editor/src/fields/CodeField.tsx. YextField no longer constructs or imports the old editor CodeField; when config.type === "code" it returns a plain field descriptor so the override system (YextPuckFields / YextPuckFieldOverrides) handles rendering. fields.ts registers the new code field and CodeFieldOverride. AdvancedSettings.tsx now renders the code field via YextAutoField. The old re-export of CodeField from the editor index was removed.

Sequence Diagram(s)

sequenceDiagram
    participant Field as CodeFieldOverride
    participant Pending as PendingSessions
    participant Parent as window.parent
    participant Listener as MessageListener
    participant Host as onChange

    Field->>Pending: generate messageId, store {id, apply}
    Field->>Parent: postMessage("constantValueEditorOpened", { id, type:"Code", value, codeLanguage, fieldName })
    Parent-->>Listener: user edits -> postMessage("constantValueEditorClosed", { id, value })
    Listener->>Pending: lookup session by id
    Pending-->>Field: invoke stored apply(payload)
    Field->>Host: call onChange(String(payload.value))
Loading

Possibly related PRs

Suggested reviewers

  • briantstephan
  • jwartofsky-yext
  • benlife5
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main refactoring objective: making the code field a real field type instead of a custom implementation.
Description check ✅ Passed The brief description 'Makes code a real fieldType' directly relates to the changeset, which involves moving CodeField from a custom editor-specific implementation to a standard field type.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch code-field

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
Copy Markdown
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

🧹 Nitpick comments (1)
packages/visual-editor/src/fields/CodeField.tsx (1)

63-88: Local development handling differs from established pattern.

Two inconsistencies with AdvancedSettings.tsx:

  1. Message ordering: The editor-open message is sent (line 70) before checking for local dev mode (line 81), meaning Storm receives the message even in local dev. In AdvancedSettings.tsx, the local dev check happens first, preventing unnecessary message dispatch.

  2. Cancel behavior: onChange(userInput ?? "") (line 83) sets the value to an empty string if the user cancels the prompt. In AdvancedSettings.tsx, canceling preserves the original value by only calling onChange when userInput !== null.

♻️ Proposed fix to align with existing pattern
   const handleClick = () => {
+    /** Handles local development testing outside of storm */
+    if (window.location.href.includes("http://localhost:5173/dev-location")) {
+      const userInput = prompt("Enter Code:", value);
+      if (userInput !== null) {
+        onChange(userInput);
+      }
+      return;
+    }
+
     const messageId = `CodeBlock-${Date.now()}`;
     pendingCodeSession = {
       messageId,
       apply: (payload) => onChange(payload.value),
     };

     openConstantValueEditor({
       payload: {
         type: "Code",
         value,
         id: messageId,
         fieldName: translatedLabel,
         codeLanguage,
       },
     });
-
-    /** Handles local development testing outside of storm */
-    if (window.location.href.includes("http://localhost:5173/dev-location")) {
-      const userInput = prompt("Enter Code:");
-      onChange(userInput ?? "");
-      if (pendingCodeSession?.messageId === messageId) {
-        pendingCodeSession = undefined;
-      }
-    }
   };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/visual-editor/src/fields/CodeField.tsx` around lines 63 - 88, The
handleClick function sends the openConstantValueEditor message even in local dev
and overwrites the field with an empty string on prompt cancel; change it to
first check the local-dev condition
(window.location.href.includes("http://localhost:5173/dev-location")) and handle
the prompt there—only call onChange when userInput !== null to preserve the
original value—and return early to avoid creating pendingCodeSession or calling
openConstantValueEditor; keep the pendingCodeSession / messageId creation and
openConstantValueEditor path for non-local runs (references: handleClick,
pendingCodeSession, openConstantValueEditor, onChange, translatedLabel) so
behavior matches AdvancedSettings.tsx.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/visual-editor/src/editor/YextField.tsx`:
- Around line 262-268: The code branch handling config.type === "code" is
missing propagation of the optional visible property; update the object returned
in that branch (the code field descriptor for type "code") to include visible:
config.visible so it behaves like the other handlers (e.g., the text branch) and
respects the CodeField visible?: boolean flag.

---

Nitpick comments:
In `@packages/visual-editor/src/fields/CodeField.tsx`:
- Around line 63-88: The handleClick function sends the openConstantValueEditor
message even in local dev and overwrites the field with an empty string on
prompt cancel; change it to first check the local-dev condition
(window.location.href.includes("http://localhost:5173/dev-location")) and handle
the prompt there—only call onChange when userInput !== null to preserve the
original value—and return early to avoid creating pendingCodeSession or calling
openConstantValueEditor; keep the pendingCodeSession / messageId creation and
openConstantValueEditor path for non-local runs (references: handleClick,
pendingCodeSession, openConstantValueEditor, onChange, translatedLabel) so
behavior matches AdvancedSettings.tsx.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f73339e1-e112-48b8-a3e7-64caaf2dffad

📥 Commits

Reviewing files that changed from the base of the PR and between c23c79a and 11c9932.

📒 Files selected for processing (8)
  • packages/visual-editor/src/docs/ai/components.d.ts
  • packages/visual-editor/src/editor/CodeField.tsx
  • packages/visual-editor/src/editor/YextField.test.tsx
  • packages/visual-editor/src/editor/YextField.tsx
  • packages/visual-editor/src/editor/index.ts
  • packages/visual-editor/src/fields/CodeField.tsx
  • packages/visual-editor/src/fields/fields.ts
  • packages/visual-editor/src/internal/components/AdvancedSettings.tsx
💤 Files with no reviewable changes (2)
  • packages/visual-editor/src/editor/index.ts
  • packages/visual-editor/src/editor/CodeField.tsx

Comment thread packages/visual-editor/src/editor/YextField.tsx
asanehisa
asanehisa previously approved these changes Apr 9, 2026
Copy link
Copy Markdown
Contributor

@asanehisa asanehisa left a comment

Choose a reason for hiding this comment

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

lgtm, i think code field was entirely watson work if u wanna tag them as well

Comment thread packages/visual-editor/src/fields/CodeField.tsx
Comment thread packages/visual-editor/src/internal/components/AdvancedSettings.tsx Outdated
Comment thread packages/visual-editor/src/docs/ai/components.d.ts
Comment thread packages/visual-editor/src/editor/YextField.tsx
Base automatically changed from basic-selector to main April 10, 2026 15:08
@mkilpatrick mkilpatrick dismissed asanehisa’s stale review April 10, 2026 15:08

The base branch was changed.

@jwartofsky-yext
Copy link
Copy Markdown
Contributor

_ No description provided. _

Can you add a description? Also there's merge conflicts

Comment thread packages/visual-editor/src/fields/BasicSelectorField.tsx Outdated
Comment thread packages/visual-editor/src/fields/BasicSelectorField.tsx Outdated
Comment thread packages/visual-editor/src/fields/YextAutoField.tsx Outdated
benlife5
benlife5 previously approved these changes Apr 10, 2026
@mkilpatrick mkilpatrick merged commit 3d81b12 into main Apr 10, 2026
14 of 17 checks passed
@mkilpatrick mkilpatrick deleted the code-field branch April 10, 2026 17:15
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.

5 participants