Migrate language widget to signals and adopt @if/@for template syntax#1534
Merged
Conversation
Migrate all three language widget components (display, record-view, edit) from decorator-based @Input/@output to Angular signal APIs (input, output, computed, signal). Update S3 widget template and CLAUDE.md coding conventions to require built-in control flow (@if, @for) over structural directives. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates the language widget components to Angular’s signal-based inputs/outputs/computed APIs and adopts the new built-in template control flow syntax across updated templates.
Changes:
- Refactors language display/record-view/edit components to use
input(),output(),computed(), andsignal(). - Updates templates to use built-in control flow (
@if,@for) instead of*ngIf/*ngFor. - Documents the template convention in
frontend/CLAUDE.md.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/app/components/ui-components/table-display-fields/language/language.component.ts | Converts display widget logic from ngOnInit state to computed signals. |
| frontend/src/app/components/ui-components/table-display-fields/language/language.component.spec.ts | Updates tests to set signal inputs via fixture.componentRef.setInput() and call computed getters. |
| frontend/src/app/components/ui-components/table-display-fields/language/language.component.html | Migrates template to @if and signal reads (languageName(), languageFlag()). |
| frontend/src/app/components/ui-components/record-view-fields/language/language.component.ts | Converts record-view widget logic to computed signals. |
| frontend/src/app/components/ui-components/record-view-fields/language/language.component.spec.ts | Updates tests for signal inputs/computed reads. |
| frontend/src/app/components/ui-components/record-view-fields/language/language.component.html | Migrates record-view template to @if and signal reads. |
| frontend/src/app/components/ui-components/record-edit-fields/s3/s3.component.html | Replaces *ngIf blocks with @if/@else if control flow for preview/open UI. |
| frontend/src/app/components/ui-components/record-edit-fields/language/language.component.ts | Migrates edit widget to signal inputs/outputs and signal-based UI state (flag). |
| frontend/src/app/components/ui-components/record-edit-fields/language/language.component.spec.ts | Updates edit widget tests for signal inputs and signal reads. |
| frontend/src/app/components/ui-components/record-edit-fields/language/language.component.html | Migrates edit widget template to @if/@for and signal reads. |
| frontend/CLAUDE.md | Adds convention requiring built-in template control flow in new code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+113
to
+124
| private loadLanguages(): void { | ||
| this.languages = LANGUAGES.map((language) => ({ | ||
| value: language.code, | ||
| label: language.name, | ||
| flag: getLanguageFlag(language), | ||
| nativeName: language.nativeName, | ||
| })).toSorted((a, b) => a.label.localeCompare(b.label)); | ||
|
|
||
| if (this.widgetStructure?.widget_params?.allow_null || this.structure?.allow_null) { | ||
| this.languages = [{ value: null, label: '', flag: '' }, ...this.languages]; | ||
| } | ||
| } | ||
| const ws = this.widgetStructure(); | ||
| if (ws?.widget_params?.allow_null || this.structure()?.allow_null) { | ||
| this.languages = [{ value: null, label: '', flag: '' }, ...this.languages]; | ||
| } |
There was a problem hiding this comment.
allow_null is read from ws.widget_params without handling the case where widget_params is a JSON string (which you already handle in showFlag). If widget_params is serialized, the null option won’t be added. Consider parsing widget_params once (e.g. a shared computed) and using that parsed object for both show_flag and allow_null.
Comment on lines
+16
to
+26
| export class LanguageDisplayComponent { | ||
| static type = 'language'; | ||
|
|
||
| public languageName: string = ''; | ||
| public languageFlag: string = ''; | ||
| public showFlag: boolean = true; | ||
| readonly key = input<string>(); | ||
| readonly value = input<any>(); | ||
| readonly structure = input<TableField>(); | ||
| readonly widgetStructure = input<WidgetStructure>(); | ||
| readonly rowData = input<Record<string, unknown>>(); | ||
| readonly primaryKeys = input<{ column_name: string }[]>(); | ||
|
|
||
| ngOnInit(): void { | ||
| this.parseWidgetParams(); | ||
| readonly onCopyToClipboard = output<string>(); |
There was a problem hiding this comment.
Unlike the other table-display field components (which all extend BaseTableDisplayFieldComponent), this component no longer extends the base class. If the base class is used as a common type/contract elsewhere, consider either updating the base to signal inputs/outputs or introducing a shared interface so language remains consistent with the rest of the display-field components.
Comment on lines
+19
to
+43
| readonly key = input<string>(); | ||
| readonly value = input<any>(); | ||
| readonly structure = input<TableField>(); | ||
| readonly widgetStructure = input<WidgetStructure>(); | ||
| readonly rowData = input<Record<string, unknown>>(); | ||
| readonly primaryKeys = input<{ column_name: string }[]>(); | ||
|
|
||
| ngOnInit(): void { | ||
| this.parseWidgetParams(); | ||
| readonly onCopyToClipboard = output<string>(); | ||
|
|
||
| if (this.value) { | ||
| const language = LANGUAGES.find(l => l.code.toLowerCase() === this.value.toLowerCase()); | ||
| this.languageName = language ? language.name : this.value; | ||
| if (language) { | ||
| this.languageFlag = getLanguageFlag(language); | ||
| } | ||
| } else { | ||
| this.languageName = '—'; | ||
| this.languageFlag = ''; | ||
| } | ||
| } | ||
| readonly showFlag = computed(() => { | ||
| const ws = this.widgetStructure(); | ||
| if (!ws?.widget_params) return true; | ||
| try { | ||
| const params = typeof ws.widget_params === 'string' ? JSON.parse(ws.widget_params) : ws.widget_params; | ||
| return params.show_flag !== undefined ? params.show_flag : true; | ||
| } catch { | ||
| return true; | ||
| } | ||
| }); | ||
|
|
||
| private parseWidgetParams(): void { | ||
| if (this.widgetStructure?.widget_params) { | ||
| try { | ||
| const params = typeof this.widgetStructure.widget_params === 'string' | ||
| ? JSON.parse(this.widgetStructure.widget_params) | ||
| : this.widgetStructure.widget_params; | ||
| readonly languageName = computed(() => { | ||
| const val = this.value(); | ||
| if (!val) return '—'; | ||
| const language = LANGUAGES.find((l) => l.code.toLowerCase() === val.toLowerCase()); | ||
| return language ? language.name : val; |
There was a problem hiding this comment.
value is declared as input<any>(), but the computed properties call val.toLowerCase(). If a non-string (e.g. number/object) is ever passed in, this will throw at runtime. Prefer typing this input as string | null | undefined and guarding/coercing before calling string methods.
Comment on lines
+15
to
+40
| readonly key = input<string>(); | ||
| readonly value = input<any>(); | ||
| readonly structure = input<TableField>(); | ||
| readonly widgetStructure = input<WidgetStructure>(); | ||
| readonly rowData = input<Record<string, unknown>>(); | ||
| readonly primaryKeys = input<Record<string, unknown>>(); | ||
|
|
||
| ngOnInit(): void { | ||
| this.parseWidgetParams(); | ||
| readonly onCopyToClipboard = output<string>(); | ||
|
|
||
| if (this.value) { | ||
| const language = LANGUAGES.find(l => l.code.toLowerCase() === this.value.toLowerCase()); | ||
| this.languageName = language ? language.name : this.value; | ||
| if (language) { | ||
| this.languageFlag = getLanguageFlag(language); | ||
| } | ||
| } else { | ||
| this.languageName = '—'; | ||
| this.languageFlag = ''; | ||
| } | ||
| } | ||
| readonly showFlag = computed(() => { | ||
| const ws = this.widgetStructure(); | ||
| if (!ws?.widget_params) return true; | ||
| try { | ||
| const params = typeof ws.widget_params === 'string' ? JSON.parse(ws.widget_params) : ws.widget_params; | ||
| return params.show_flag !== undefined ? params.show_flag : true; | ||
| } catch { | ||
| return true; | ||
| } | ||
| }); | ||
|
|
||
| private parseWidgetParams(): void { | ||
| if (this.widgetStructure?.widget_params) { | ||
| try { | ||
| const params = typeof this.widgetStructure.widget_params === 'string' | ||
| ? JSON.parse(this.widgetStructure.widget_params) | ||
| : this.widgetStructure.widget_params; | ||
| readonly languageName = computed(() => { | ||
| const val = this.value(); | ||
| if (!val) return '—'; | ||
| const language = LANGUAGES.find((l) => l.code.toLowerCase() === val.toLowerCase()); | ||
| return language ? language.name : val; | ||
| }); |
There was a problem hiding this comment.
value is declared as input<any>(), but languageName/languageFlag call val.toLowerCase(). If value is not a string, this will throw. Tighten the input type (e.g. string | null) and guard/coerce before using string methods.
Comment on lines
1
to
+8
| import { CommonModule } from '@angular/common'; | ||
| import { Component, computed, input, output } from '@angular/core'; | ||
| import { TableField, WidgetStructure } from 'src/app/models/table'; | ||
| import { getLanguageFlag, LANGUAGES } from '../../../../consts/languages'; | ||
|
|
||
| @Injectable() | ||
| @Component({ | ||
| selector: 'app-language-record-view', | ||
| templateUrl: './language.component.html', | ||
| styleUrls: ['../base-record-view-field/base-record-view-field.component.css', './language.component.css'], | ||
| imports: [CommonModule] | ||
| selector: 'app-language-record-view', | ||
| templateUrl: './language.component.html', |
There was a problem hiding this comment.
All other record-view field components are annotated with @Injectable() (e.g. record-view-fields/country/country.component.ts:7). This one removed it, which is inconsistent and may break any code relying on injecting these field components. Re-add @Injectable() (and its import) for consistency with the rest of the record-view fields.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrate all three language widget components (display, record-view, edit) from decorator-based @Input/@output to Angular signal APIs (input, output, computed, signal). Update S3 widget template and CLAUDE.md coding conventions to require built-in control flow (@if, @for) over structural directives.