Skip to content

Commit 332d64a

Browse files
josemontespgjosemontespg
authored
refactor(angular): use declarative HTML for known Text variants (a2ui-project#1502)
What Refactors Angular 0.9 Text component to render headings (h1-h5) and caption variants using declarative HTML elements in the template with {{ text }} notation instead of using the markdown renderer. Markdown rendering is still used for unknown/default variants (like 'body'). Why? In 1P code, there are some builds that don't allow the g3mark implementation of the markdown renderer, due to a strict CSP (blocking 'unsafe-eval'). The only reason to provide a markdown renderer is because the Text component requires some kind of markdown renderer, even to display simple variants like headings. With the changes in this PR, apps can use provideMarkdownRenderer(async text => text) and still get all simple variants working correctly. Tests have been updated to assert this new behavior.
1 parent e05dd96 commit 332d64a

2 files changed

Lines changed: 87 additions & 34 deletions

File tree

‎renderers/angular/src/v0_9/catalog/basic/text.component.spec.ts‎

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,11 @@ describe('TextComponent', () => {
6060
await fixture.whenStable();
6161
fixture.detectChanges();
6262

63-
const span = fixture.debugElement.query(By.css('span'));
64-
expect(span.nativeElement.innerHTML.trim()).toBe('<p>Hello World</p>');
63+
const element = fixture.debugElement.query(By.css('.a2ui-text'));
64+
expect(element).toBeTruthy();
65+
const p = element.query(By.css('p'));
66+
expect(p).toBeTruthy();
67+
expect(p.nativeElement.textContent.trim()).toBe('Hello World');
6568
expect(mockMarkdownRenderer.render).toHaveBeenCalledWith('Hello World');
6669
});
6770

@@ -74,7 +77,12 @@ describe('TextComponent', () => {
7477
await fixture.whenStable();
7578
fixture.detectChanges();
7679

77-
expect(mockMarkdownRenderer.render).toHaveBeenCalledWith('# Heading');
80+
expect(mockMarkdownRenderer.render).not.toHaveBeenCalled();
81+
const element = fixture.debugElement.query(By.css('.a2ui-text.h1'));
82+
expect(element).toBeTruthy();
83+
const h1 = element.query(By.css('h1'));
84+
expect(h1).toBeTruthy();
85+
expect(h1.nativeElement.textContent.trim()).toBe('Heading');
7886
});
7987

8088
it('should handle variant caption', async () => {
@@ -86,7 +94,12 @@ describe('TextComponent', () => {
8694
await fixture.whenStable();
8795
fixture.detectChanges();
8896

89-
expect(mockMarkdownRenderer.render).toHaveBeenCalledWith('*Caption*');
97+
expect(mockMarkdownRenderer.render).not.toHaveBeenCalled();
98+
const element = fixture.debugElement.query(By.css('.a2ui-text.caption'));
99+
expect(element).toBeTruthy();
100+
const em = element.query(By.css('em'));
101+
expect(em).toBeTruthy();
102+
expect(em.nativeElement.textContent.trim()).toBe('Caption');
90103
});
91104

92105
it('should handle variant h2', async () => {
@@ -98,7 +111,12 @@ describe('TextComponent', () => {
98111
await fixture.whenStable();
99112
fixture.detectChanges();
100113

101-
expect(mockMarkdownRenderer.render).toHaveBeenCalledWith('## Heading');
114+
expect(mockMarkdownRenderer.render).not.toHaveBeenCalled();
115+
const element = fixture.debugElement.query(By.css('.a2ui-text.h2'));
116+
expect(element).toBeTruthy();
117+
const h2 = element.query(By.css('h2'));
118+
expect(h2).toBeTruthy();
119+
expect(h2.nativeElement.textContent.trim()).toBe('Heading');
102120
});
103121

104122
it('should handle variant h3', async () => {
@@ -110,7 +128,12 @@ describe('TextComponent', () => {
110128
await fixture.whenStable();
111129
fixture.detectChanges();
112130

113-
expect(mockMarkdownRenderer.render).toHaveBeenCalledWith('### Heading');
131+
expect(mockMarkdownRenderer.render).not.toHaveBeenCalled();
132+
const element = fixture.debugElement.query(By.css('.a2ui-text.h3'));
133+
expect(element).toBeTruthy();
134+
const h3 = element.query(By.css('h3'));
135+
expect(h3).toBeTruthy();
136+
expect(h3.nativeElement.textContent.trim()).toBe('Heading');
114137
});
115138

116139
it('should handle variant h4', async () => {
@@ -122,7 +145,12 @@ describe('TextComponent', () => {
122145
await fixture.whenStable();
123146
fixture.detectChanges();
124147

125-
expect(mockMarkdownRenderer.render).toHaveBeenCalledWith('#### Heading');
148+
expect(mockMarkdownRenderer.render).not.toHaveBeenCalled();
149+
const element = fixture.debugElement.query(By.css('.a2ui-text.h4'));
150+
expect(element).toBeTruthy();
151+
const h4 = element.query(By.css('h4'));
152+
expect(h4).toBeTruthy();
153+
expect(h4.nativeElement.textContent.trim()).toBe('Heading');
126154
});
127155

128156
it('should handle variant h5', async () => {
@@ -134,7 +162,12 @@ describe('TextComponent', () => {
134162
await fixture.whenStable();
135163
fixture.detectChanges();
136164

137-
expect(mockMarkdownRenderer.render).toHaveBeenCalledWith('##### Heading');
165+
expect(mockMarkdownRenderer.render).not.toHaveBeenCalled();
166+
const element = fixture.debugElement.query(By.css('.a2ui-text.h5'));
167+
expect(element).toBeTruthy();
168+
const h5 = element.query(By.css('h5'));
169+
expect(h5).toBeTruthy();
170+
expect(h5.nativeElement.textContent.trim()).toBe('Heading');
138171
});
139172

140173
it('should handle missing text property', async () => {

‎renderers/angular/src/v0_9/catalog/basic/text.component.ts‎

Lines changed: 46 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,35 @@ import {TextApi} from '@a2ui/web_core/v0_9/basic_catalog';
3838
@Component({
3939
selector: 'a2ui-v09-text',
4040
standalone: true,
41-
template: ` <span [class]="'a2ui-text ' + variant()" [innerHTML]="resolvedText()"> </span> `,
41+
template: `
42+
@if (isNonMarkdownVariant()) {
43+
<span [class]="'a2ui-text ' + variant()">
44+
<!-- Nesting block elements like h1 inside a span is not correct HTML5. We should refactor this-->
45+
@switch (variant()) {
46+
@case ('h1') {
47+
<h1>{{ text() }}</h1>
48+
}
49+
@case ('h2') {
50+
<h2>{{ text() }}</h2>
51+
}
52+
@case ('h3') {
53+
<h3>{{ text() }}</h3>
54+
}
55+
@case ('h4') {
56+
<h4>{{ text() }}</h4>
57+
}
58+
@case ('h5') {
59+
<h5>{{ text() }}</h5>
60+
}
61+
@case ('caption') {
62+
<em>{{ text() }}</em>
63+
}
64+
}
65+
</span>
66+
} @else {
67+
<span [class]="'a2ui-text ' + variant()" [innerHTML]="resolvedText()"></span>
68+
}
69+
`,
4270
// We use :host ::ng-deep because the template content is injected via innerHTML (Markdown).
4371
// Angular's default view encapsulation cannot target elements injected via innerHTML because they lack the scoping attributes generated at compile time.
4472
// ::ng-deep allows styles to reach into the injected HTML, while :host keeps them scoped to this component.
@@ -107,42 +135,34 @@ import {TextApi} from '@a2ui/web_core/v0_9/basic_catalog';
107135
export class TextComponent extends BasicCatalogComponent<typeof TextApi> {
108136
private markdownRenderer = inject(MarkdownRenderer);
109137

138+
private static readonly NON_MARKDOWN_VARIANTS = new Set<string>([
139+
'h1',
140+
'h2',
141+
'h3',
142+
'h4',
143+
'h5',
144+
'caption',
145+
]);
146+
110147
readonly variant = computed(() => this.props()['variant']?.value() || 'body');
111148
readonly text = computed(() => this.props()['text']?.value() || '');
112149

150+
readonly isNonMarkdownVariant = computed(() => {
151+
return TextComponent.NON_MARKDOWN_VARIANTS.has(this.variant());
152+
});
153+
113154
resolvedText = signal<string>('');
114155
private renderRequestId = 0;
115156

116157
constructor() {
117158
super();
118159
effect(() => {
119-
const text = this.text();
120-
const variant = this.variant();
121-
let value = text;
122-
123-
switch (variant) {
124-
case 'h1':
125-
value = `# ${text}`;
126-
break;
127-
case 'h2':
128-
value = `## ${text}`;
129-
break;
130-
case 'h3':
131-
value = `### ${text}`;
132-
break;
133-
case 'h4':
134-
value = `#### ${text}`;
135-
break;
136-
case 'h5':
137-
value = `##### ${text}`;
138-
break;
139-
case 'caption':
140-
value = `*${text}*`;
141-
break;
160+
if (this.isNonMarkdownVariant()) {
161+
return;
142162
}
143-
163+
const text = this.text();
144164
const requestId = ++this.renderRequestId;
145-
this.markdownRenderer.render(value).then(rendered => {
165+
this.markdownRenderer.render(text).then(rendered => {
146166
if (requestId === this.renderRequestId) {
147167
this.resolvedText.set(rendered);
148168
}

0 commit comments

Comments
 (0)