fix[gen2-sdks]: able to edit and preview when we have no content (#3726)

## Description Jira https://builder-io.atlassian.net/browse/PLAN-338 Fixes #3653 _Screenshot_ If relevant, add a screenshot or two of the changes you made.
BuilderIO · Dec 19, 2024 · ebfd876 · ebfd876
1 parent f37a36f
commit ebfd876
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 5 deletions.
diff --git a/.changeset/slimy-rivers-boil.md b/.changeset/slimy-rivers-boil.md
@@ -0,0 +1,7 @@
+---
+"@builder.io/sdk-qwik": patch
+"@builder.io/sdk-svelte": patch
+"@builder.io/sdk-vue": patch
+---
+
+Fix: allow previewing and visual editing when content is null
diff --git a/packages/sdks-tests/src/e2e-tests/editing-empty-content.spec.ts b/packages/sdks-tests/src/e2e-tests/editing-empty-content.spec.ts
@@ -0,0 +1,38 @@
+import { expect } from '@playwright/test';
+import { test } from '../helpers/index.js';
+import { launchEmbedderAndWaitForSdk, sendContentUpdateMessage } from '../helpers/visual-editor.js';
+import { CONTENT } from '../specs/text-block.js';
+import type { Sdk } from '../helpers/sdk.js';
+
+test.describe('Editing empty content', () => {
+  /**
+   * We are ignoring hydration warnings here because this occurs in a specific scenario where we are either editing or previewing
+   * and `content` provided is `null`. In this case, the `div` for `EnableEditor` is only attached on the client.
+   */
+  test.use({ ignoreHydrationErrors: true });
+
+  test('should be able to edit when content prop is not provided', async ({
+    page,
+    sdk,
+    basePort,
+  }) => {
+    const sdksUsingElementRefApproach = ['svelte', 'vue', 'qwik'] as Sdk[];
+    test.skip(!sdksUsingElementRefApproach.includes(sdk));
+
+    await launchEmbedderAndWaitForSdk({
+      // special page added only in nuxt e2e that doesn't pass `content`
+      path: '/editing-empty-content-element-ref',
+      page,
+      sdk,
+      basePort,
+    });
+
+    await sendContentUpdateMessage({
+      page,
+      newContent: CONTENT,
+      model: 'page',
+    });
+
+    await expect(page.frameLocator('iframe').getByText('Start of text box.')).toBeVisible();
+  });
+});
diff --git a/packages/sdks-tests/src/helpers/index.ts b/packages/sdks-tests/src/helpers/index.ts
@@ -15,6 +15,7 @@ type TestOptions = {
   packageName: ServerName;
   sdk: Sdk;
   basePort: number;
+  ignoreHydrationErrors: boolean;
 };
 
 // https://github.com/microsoft/playwright/issues/14854#issuecomment-1155667859
@@ -41,7 +42,8 @@ const test = base.extend<TestOptions>({
   packageName: ['DEFAULT' as any, { option: true }],
   sdk: ['DEFAULT' as any, { option: true }],
   basePort: [0, { option: true }],
-  page: async ({ context, page, packageName, sdk }, use) => {
+  ignoreHydrationErrors: [false, { option: true }],
+  page: async ({ context, page, packageName, sdk, ignoreHydrationErrors }, use) => {
     if (packageName === ('DEFAULT' as any)) {
       throw new Error('packageName is required');
     }
@@ -61,7 +63,7 @@ const test = base.extend<TestOptions>({
     /**
      * temporarily disable hydration error checks for hydrogen until we fix them.
      */
-    const shouldCheckForHydrationError = packageName !== 'hydrogen';
+    const shouldCheckForHydrationError = packageName !== 'hydrogen' && !ignoreHydrationErrors;
 
     if (shouldCheckForHydrationError) {
       context.on('console', msg => {

diff --git a/packages/sdks-tests/src/specs/index.ts b/packages/sdks-tests/src/specs/index.ts
@@ -196,6 +196,10 @@ export const PAGES: Record<string, Page> = {
   '/localization-locale-not-passed': { content: LOCALIZATION_WITHOUT_LOCALE_PROP },
   '/localization-subfields': { content: LOCALIZATION_SUBFIELDS },
   '/get-content-with-symbol': { content: CONTENT_WITHOUT_SYMBOLS, target: 'gen1' },
+  '/editing-empty-content-element-ref': {
+    content: null as unknown as BuilderContent,
+    target: ['svelte', 'sveltekit', 'vue', 'nuxt', 'qwik-city'],
+  },
 } as const;
 
 export type Path = keyof typeof PAGES;

diff --git a/packages/sdks/e2e/nuxt/pages/[...app].vue b/packages/sdks/e2e/nuxt/pages/[...app].vue
@@ -56,7 +56,7 @@ const { data: props } = await useAsyncData('builderData', async () => {
 </script>
 
 <template>
-  <div v-if="props?.content">
+  <div v-if="props">
     <Content v-bind="props" :customComponents="REGISTERED_COMPONENTS" />
   </div>
   <div v-else>Content not Found</div>

diff --git a/packages/sdks/e2e/vue/src/App.vue b/packages/sdks/e2e/vue/src/App.vue
@@ -59,7 +59,7 @@ export default defineComponent({
 </script>
 
 <template>
-  <div v-if="props?.content">
+  <div v-if="props">
     <builder-render-content
       v-bind="props"
       :customComponents="registeredComponents"

diff --git a/packages/sdks/src/components/content/components/enable-editor.lite.tsx b/packages/sdks/src/components/content/components/enable-editor.lite.tsx
@@ -441,7 +441,17 @@ export default function EnableEditor(props: BuilderEditorProps) {
   }, [props.locale]);
 
   return (
-    <Show when={props.builderContextSignal.value.content}>
+    <Show
+      when={
+        /**
+         * We need to attach this div only when content exists or isPreviewing/isEditing even when content is null,
+         * as we need to set the elementRef and allow previewing and visual editing
+         */
+        props.builderContextSignal.value.content ||
+        isPreviewing() ||
+        isEditing()
+      }
+    >
       <state.ContentWrapper
         {...useTarget({
           qwik: {
@@ -463,6 +473,16 @@ export default function EnableEditor(props: BuilderEditorProps) {
         className={getWrapperClassName(
           props.content?.testVariationId || props.content?.id
         )}
+        // content exists: render div and display: undefined
+        // content does not exist but isEditing/isPreviewing: render div and display: 'none'
+        // once inline editing kicks in, it will populate the content and re-render, so display style will be removed
+        style={{
+          display:
+            !props.builderContextSignal.value.content &&
+            (isPreviewing() || isEditing())
+              ? 'none'
+              : undefined,
+        }}
         {...useTarget({
           reactNative: {
             // currently, we can't set the actual ID here.