Skip to content

Conversation

@freesbi
Copy link
Collaborator

@freesbi freesbi commented Apr 1, 2025

Description

Refactored ReadonlyFormField and ReadonlyFormFieldWrapper, because of performance issues.
There was several performance issues with both of the components.

  • Removed AfterViewChecked lifecycle hook hard usage
  • Changed Change Detection Strategy
  • DOM re-render logic moved and optimised
  • In case of ReadonlyFormFieldWrapper cdkContentObserver was removed, because produce a lot of UI locks and performance issues
  • Several issues fixed
  • Added API specifications for Demo application
  • Rework of Demo Page
  • Added Jest unit tests for both components

Alert icons styles allignment

Which Component is affected or generated?

  • mad-readonly-form-field
  • mad-readonly-form-field-wrapper
  • mad-alert

@Input('unitPosition') unitPosition: 'right' | 'left' = 'left';
@Input('errorMessage') errorMessage: string | null = null;
@Input() id: string;
@Input() id!: string;
Copy link

@LisaKirchhofer LisaKirchhofer Apr 28, 2025

Choose a reason for hiding this comment

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

Here and everywhere else: I think we should use

Suggested change
@Input() id!: string;
@Input({ required: true }) id: string;

instead :)

this.doRendering();
ngOnChanges(_: SimpleChanges): void {
if (this.initialized) {
this.syncView();

Choose a reason for hiding this comment

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

Why do we need this?

export class ReadOnlyFormFieldComponent implements OnChanges, AfterViewInit {
@Input() useProjectedContent: boolean = false;
@Input() value?: any;
@Input() label!: string;

Choose a reason for hiding this comment

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

This & other files, same as above:

Suggested change
@Input() label!: string;
@Input({ required: true }) label: string;

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.

3 participants