Skip to content

Commit f4ff27d

Browse files
yash-builderYash Wadhiasamijaber
authored
fix[dynamic-renderer]: ENG-8440 Button links cannot be used in Angular Gen 2 SDK (BuilderIO#3937)
## Description This PR replaces the switch-case logic with a Map for dynamically selecting components in `dynamic-renderer`. The previous approach caused an assertion error when users typed. _Loom_ https://www.loom.com/share/1229e7974db44743921df588ca1bd1bb?sid=ea3ccc9c-41de-4efe-b499-f61ca9244cbd --------- Co-authored-by: Yash Wadhia <[email protected]> Co-authored-by: Sami Jaber <[email protected]>
1 parent 4fd2e14 commit f4ff27d

File tree

5 files changed

+132
-8
lines changed

5 files changed

+132
-8
lines changed

.changeset/loud-jars-sleep.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@builder.io/sdk-angular": patch
3+
---
4+
5+
Fix: crashes when visually editing blocks (encountered when SDK dynamically switched HTML elements)
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { expect } from '@playwright/test';
2+
import { checkIsRN, test } from '../helpers/index.js';
3+
import {
4+
cloneContent,
5+
launchEmbedderAndWaitForSdk,
6+
sendPatchOrUpdateMessage,
7+
} from '../helpers/visual-editor.js';
8+
import { DYNAMIC_BUTTON } from '../specs/dynamic-button.js';
9+
10+
test.describe('Dynamic Button', () => {
11+
test('should render a button', async ({ page, sdk, basePort, packageName }) => {
12+
13+
test.fail(sdk === 'svelte' || sdk === 'oldReact', 'Not showing the href attribute in Svelte');
14+
test.skip(
15+
packageName === 'nextjs-sdk-next-app' ||
16+
packageName === 'gen1-next14-pages' ||
17+
packageName === 'gen1-next15-app' ||
18+
packageName === 'gen1-remix'
19+
);
20+
await launchEmbedderAndWaitForSdk({
21+
path: '/dynamic-button',
22+
basePort,
23+
page,
24+
sdk,
25+
});
26+
27+
const buttonLocator = checkIsRN(sdk) ? page.frameLocator('iframe').locator('button') : page
28+
.frameLocator('iframe')
29+
.locator('[builder-id="builder-b53d1cc2bcbb481b869207fdd97ee1db"]');
30+
31+
await expect(buttonLocator).toHaveText('Click me!');
32+
const newContent = cloneContent(DYNAMIC_BUTTON);
33+
34+
// simulating typing in the link field
35+
await sendPatchOrUpdateMessage({
36+
page,
37+
content: cloneContent(DYNAMIC_BUTTON),
38+
model: 'page',
39+
sdk,
40+
updateFn: () => '#',
41+
path: '/data/blocks/0/component/options/link',
42+
});
43+
44+
await sendPatchOrUpdateMessage({
45+
page,
46+
content: newContent,
47+
model: 'page',
48+
sdk,
49+
updateFn: () => '#g',
50+
path: '/data/blocks/0/component/options/link',
51+
});
52+
53+
await sendPatchOrUpdateMessage({
54+
page,
55+
content: newContent,
56+
model: 'page',
57+
sdk,
58+
updateFn: () => '#go',
59+
path: '/data/blocks/0/component/options/link',
60+
});
61+
62+
const updatedButtonLocator = checkIsRN(sdk) ? page.frameLocator('iframe').locator('a') : page
63+
.frameLocator('iframe')
64+
.locator('[builder-id="builder-b53d1cc2bcbb481b869207fdd97ee1db"]');
65+
66+
await expect(updatedButtonLocator).toBeVisible();
67+
68+
await expect(updatedButtonLocator).toHaveAttribute('href', '#go');
69+
});
70+
});
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
export const DYNAMIC_BUTTON = {
2+
data: {
3+
title: 'dynamic-button-sdk-test',
4+
themeId: false,
5+
blocks: [
6+
{
7+
'@type': '@builder.io/sdk:Element',
8+
'@version': 2,
9+
id: 'builder-b53d1cc2bcbb481b869207fdd97ee1db',
10+
component: {
11+
name: 'Core:Button',
12+
options: {
13+
text: 'Click me!',
14+
openLinkInNewTab: false,
15+
},
16+
},
17+
responsiveStyles: {
18+
large: {
19+
display: 'flex',
20+
flexDirection: 'column',
21+
position: 'relative',
22+
flexShrink: '0',
23+
boxSizing: 'border-box',
24+
marginTop: '20px',
25+
appearance: 'none',
26+
paddingTop: '15px',
27+
paddingBottom: '15px',
28+
paddingLeft: '25px',
29+
paddingRight: '25px',
30+
backgroundColor: 'black',
31+
color: 'white',
32+
borderRadius: '4px',
33+
textAlign: 'center',
34+
cursor: 'pointer',
35+
},
36+
},
37+
},
38+
],
39+
},
40+
};

packages/sdks-tests/src/specs/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ import { VIDEO_LAZY_LOAD } from './video-lazy-load.js';
8787
import { COLUMNS_VERTICAL_CENTER_FLEX } from './columns-vertical-center-flex.js';
8888
import { DYNAMIC_ELEMENT } from './dynamic-element.js';
8989
import { CUSTOM_CODE_DOM_UPDATE } from './custom-code-dom-update.js';
90-
90+
import { DYNAMIC_BUTTON } from './dynamic-button.js';
9191
function isBrowser(): boolean {
9292
return typeof window !== 'undefined' && typeof document !== 'undefined';
9393
}
@@ -279,6 +279,7 @@ export const PAGES: Record<string, Page> = {
279279
'/can-track-false-pre-init': { content: HOMEPAGE, target: 'gen1' },
280280
'/dynamic-element': { content: DYNAMIC_ELEMENT },
281281
'/custom-code-dom-update': { content: CUSTOM_CODE_DOM_UPDATE },
282+
'/dynamic-button': { content: DYNAMIC_BUTTON },
282283
} as const;
283284

284285
export type Path = keyof typeof PAGES;

packages/sdks/output/angular/scripts/generate-dynamic-renderer.mjs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ const dynamicComponentTemplate = (tagName) => {
7676
export class Dynamic${capitalize(tagName)} {
7777
@Input() attributes!: any;
7878
@Input() actionAttributes?: any;
79+
@Input() tagName?: string;
7980
@ViewChild('v', { read: ElementRef }) v!: ElementRef;
8081
_listenerFns = new Map<string, () => void>();
8182
constructor(private renderer: Renderer2) {}
@@ -121,7 +122,7 @@ const generateComponents = () => {
121122
<ng-container *ngIf="useTypeOf(TagName) === 'string'">
122123
<ng-container
123124
*ngComponentOutlet="
124-
TagName;
125+
getComponentType(TagName);
125126
inputs: {
126127
attributes: attributes,
127128
actionAttributes: actionAttributes,
@@ -196,14 +197,21 @@ export default class DynamicRenderer {
196197
197198
constructor(private vcRef: ViewContainerRef) {}
198199
200+
private tagComponentMap: { [key: string]: any } = {
201+
${htmlElements.map((el) => `'${el}': Dynamic${capitalize(el)}`).join(',\n ')}
202+
};
203+
204+
getComponentType(tagName: string): any {
205+
return this.tagComponentMap[tagName] || null;
206+
}
207+
199208
ngOnInit() {
200209
if (typeof this.TagName === 'string') {
201-
switch (this.TagName) {
202-
${htmlElements.map((el) => `case '${el}': this.TagName = Dynamic${capitalize(el)}; break;`).join('\n ')}
203-
default:
204-
this.tagName = this.TagName;
205-
this.TagName = DynamicElement;
206-
break;
210+
if (this.tagComponentMap[this.TagName]) {
211+
this.TagName = this.tagComponentMap[this.TagName];
212+
} else {
213+
this.tagName = this.TagName;
214+
this.TagName = DynamicElement;
207215
}
208216
}
209217
this.myContent = [this.vcRef.createEmbeddedView(this.tagnameTemplateRef).rootNodes];

0 commit comments

Comments
 (0)