refactor: create a dynamic empty content component#1985
Merged
Conversation
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The manual setTimeout + cdr.detectChanges in DynamicMatTableComponent feels like a workaround—consider using a more robust lifecycle hook or OnPush change detection strategy instead of an arbitrary delay.
- EmptyContentComponent currently only accepts static strings for messages and icons; consider supporting translation keys or integrating with your i18n/TranslatePipe so the content can be localized.
- If you need richer empty states in the future, think about adding content projection or a custom template input to EmptyContentComponent so consumers can inject complex markup beyond a simple icon and text.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The manual setTimeout + cdr.detectChanges in DynamicMatTableComponent feels like a workaround—consider using a more robust lifecycle hook or OnPush change detection strategy instead of an arbitrary delay.
- EmptyContentComponent currently only accepts static strings for messages and icons; consider supporting translation keys or integrating with your i18n/TranslatePipe so the content can be localized.
- If you need richer empty states in the future, think about adding content projection or a custom template input to EmptyContentComponent so consumers can inject complex markup beyond a simple icon and text.
## Individual Comments
### Comment 1
<location> `src/app/shared/modules/dynamic-material-table/table/dynamic-mat-table.component.ts:463` </location>
<code_context>
this.init = true;
- }, 1000);
+ this.cdr.detectChanges();
+ }, 10);
if (this.dataSource) {
</code_context>
<issue_to_address>
Consider reducing the setTimeout delay further or refactoring to avoid arbitrary timeouts for initialization.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| this.init = true; | ||
| }, 1000); | ||
| this.cdr.detectChanges(); | ||
| }, 10); |
Contributor
There was a problem hiding this comment.
issue (complexity): Consider reducing the setTimeout delay further or refactoring to avoid arbitrary timeouts for initialization.
Junjiequan
reviewed
Sep 10, 2025
Comment on lines
+91
to
+95
| <app-empty-content | ||
| message="No logbook available." | ||
| icon="book" | ||
| > | ||
| </app-empty-content> |
Member
There was a problem hiding this comment.
This indentation looks not right here?
Comment on lines
+93
to
+97
| <app-empty-content | ||
| message="No logbook available." | ||
| icon="book" | ||
| > | ||
| </app-empty-content> |
Junjiequan
approved these changes
Sep 11, 2025
Junjiequan
added a commit
that referenced
this pull request
Sep 19, 2025
This PR introduces a reusable `EmptyContentComponent` to make visual feedback consistent when no data is available with dynamic messages and icons. Background on use case, changes needed Please provide a list of the fixes implemented in this PR * https://jira.ess.eu/browse/SWAP-3168 Please provide a list of the changes implemented by this PR * changes made - [ ] Included for each change/fix? - [ ] Passing? (Merge will not be approved unless this is checked) - [ ] swagger documentation updated \[required\] - [ ] official documentation updated \[nice-to-have\] If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included - [ ] Does it require a specific version of the backend - which version of the backend is required: Introduce a reusable EmptyContentComponent for consistent empty-state UI and replace all inline no-data placeholders across tables and dashboards with this new component. New Features: - Add EmptyContentComponent with customizable message, icon, reload, and action buttons Enhancements: - Extend DynamicMatTableComponent to support emptyMessage and emptyIcon inputs and improve its change detection timing - Replace existing hardcoded no-data markup in dynamic tables, logbooks, proposals, datasets, files, and instruments dashboards with EmptyContentComponent Build: - Import EmptyContentModule in shared and DynamicMatTable modules --------- Co-authored-by: Jay <b331998513@gmail.com>
Junjiequan
added a commit
that referenced
this pull request
Sep 19, 2025
This PR introduces a reusable `EmptyContentComponent` to make visual feedback consistent when no data is available with dynamic messages and icons. Background on use case, changes needed Please provide a list of the fixes implemented in this PR * https://jira.ess.eu/browse/SWAP-3168 Please provide a list of the changes implemented by this PR * changes made - [ ] Included for each change/fix? - [ ] Passing? (Merge will not be approved unless this is checked) - [ ] swagger documentation updated \[required\] - [ ] official documentation updated \[nice-to-have\] If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included - [ ] Does it require a specific version of the backend - which version of the backend is required: Introduce a reusable EmptyContentComponent for consistent empty-state UI and replace all inline no-data placeholders across tables and dashboards with this new component. New Features: - Add EmptyContentComponent with customizable message, icon, reload, and action buttons Enhancements: - Extend DynamicMatTableComponent to support emptyMessage and emptyIcon inputs and improve its change detection timing - Replace existing hardcoded no-data markup in dynamic tables, logbooks, proposals, datasets, files, and instruments dashboards with EmptyContentComponent Build: - Import EmptyContentModule in shared and DynamicMatTable modules --------- Co-authored-by: Jay <b331998513@gmail.com>
Junjiequan
added a commit
that referenced
this pull request
Sep 19, 2025
This PR introduces a reusable `EmptyContentComponent` to make visual feedback consistent when no data is available with dynamic messages and icons. Background on use case, changes needed Please provide a list of the fixes implemented in this PR * https://jira.ess.eu/browse/SWAP-3168 Please provide a list of the changes implemented by this PR * changes made - [ ] Included for each change/fix? - [ ] Passing? (Merge will not be approved unless this is checked) - [ ] swagger documentation updated \[required\] - [ ] official documentation updated \[nice-to-have\] If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included - [ ] Does it require a specific version of the backend - which version of the backend is required: Introduce a reusable EmptyContentComponent for consistent empty-state UI and replace all inline no-data placeholders across tables and dashboards with this new component. New Features: - Add EmptyContentComponent with customizable message, icon, reload, and action buttons Enhancements: - Extend DynamicMatTableComponent to support emptyMessage and emptyIcon inputs and improve its change detection timing - Replace existing hardcoded no-data markup in dynamic tables, logbooks, proposals, datasets, files, and instruments dashboards with EmptyContentComponent Build: - Import EmptyContentModule in shared and DynamicMatTable modules --------- Co-authored-by: Jay <b331998513@gmail.com>
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
2 participants
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.
Description
This PR introduces a reusable
EmptyContentComponentto make visual feedback consistent when no data is available with dynamic messages and icons.Motivation
Background on use case, changes needed
Fixes:
Please provide a list of the fixes implemented in this PR
Changes:
Please provide a list of the changes implemented by this PR
Tests included
Documentation
official documentation info
If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included
Backend version
Summary by Sourcery
Introduce a reusable EmptyContentComponent for consistent empty-state UI and replace all inline no-data placeholders across tables and dashboards with this new component.
New Features:
Enhancements:
Build: