Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dynamic component input value assignment #1793

Merged
merged 16 commits into from
Mar 30, 2023
Merged

Dynamic component input value assignment #1793

merged 16 commits into from
Mar 30, 2023

Conversation

TheSlimvReal
Copy link
Collaborator

@TheSlimvReal TheSlimvReal commented Mar 22, 2023 •
edited
Loading

For some time we have already been planning to replace the OnInitDynamicComponent interface with a more Angular-like approach where we make use of the class properties marked with the @Input() annotation.

This has been implemented here.

Now the components that are dynamically created just work like they are used in a template where @Input() properties are automatically set and available in the ngOnInit or even ngOnChanges functions.

Visible/Frontend Changes

--

Architectural/Backend Changes

  • DynamicComponentDirective automatically assigns config values to component properties annotated with @Input()
  • removed OnInitDynamicComponent interface

@github-actions
Copy link
Contributor

Deployed to https://pr-1793.aam-digital.net/

@TheSlimvReal TheSlimvReal requested a review from sleidig March 22, 2023 12:41
@TheSlimvReal TheSlimvReal mentioned this pull request Mar 23, 2023
Merged
4 tasks
@TheSlimvReal TheSlimvReal force-pushed the dynamic_component_input branch from afa0f78 to 116b0f3 Compare March 27, 2023 07:10
sleidig
sleidig requested changes Mar 28, 2023
View reviewed changes
Copy link
Member

@sleidig sleidig left a comment

Choose a reason for hiding this comment

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

😍 😻
I absolutely love this refactoring - feels so much clearer and can avoid duplicated and convoluted code.

I commented a few places where I would prefer to have non-nested, more clear Inputs instead of the config property. Maybe a two-way with the config implemented as a setter can be a workaround/fallback?

...roject/children/children-list/recent-attendance-blocks/recent-attendance-blocks.component.ts
filterByActivityType: string;
child: Child;
@Input() entity: Child;
@Input() config: { filterByActivityType: string };
Copy link
Member

Choose a reason for hiding this comment

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

let's make this the more clean direct input @Input() filterByActivityType: string;. I think the block is currently not used anywhere due to performance issues anyway, so the backward compatibility could be sacrificed here.

src/app/features/historical-data/historical-data/historical-data.component.ts
export class HistoricalDataComponent implements OnInitDynamicComponent {
export class HistoricalDataComponent implements OnInit {
@Input() entity: Entity;
@Input() config: FormFieldConfig[] = [];
Copy link
Member

Choose a reason for hiding this comment

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

maybe "columns" is more clear and descriptive - so we could also consider doing just a .config setter as an alternative route

@TheSlimvReal
Copy link
Collaborator Author

TheSlimvReal commented Mar 28, 2023 •
edited
Loading

TODO:

  • check if ngOnChanges is triggered automatically or remove where unnecessary and use ngOnInit instead
  • flatten configs for panels in EntityDetails with fallback in case a entity name is used twice

@TheSlimvReal TheSlimvReal force-pushed the dynamic_component_input branch from ccdff23 to ccd388d Compare March 29, 2023 09:45
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug D 3 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@TheSlimvReal TheSlimvReal merged commit 3e79241 into master Mar 30, 2023
@TheSlimvReal TheSlimvReal deleted the dynamic_component_input branch March 30, 2023 17:28
@aam-digital-ci
Copy link
Collaborator

🎉 This PR is included in version 3.20.0-master.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@aam-digital-ci
Copy link
Collaborator

🎉 This PR is included in version 3.20.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sleidig sleidig sleidig approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TheSlimvReal @aam-digital-ci @sleidig