capella-collab-manager
capella-collab-manager copied to clipboard
refactor: Apply new Angular control flow
This PR applies the new Angular control flow introduced with Angular 17 to several components. This includes some simplifications that can now be applied more easily. In addition, some css classes have been converted to tailwind and unused css files have been removed. Last but not least, some line breaks and indentations in the components have been adjusted to improve readability.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 76.94%. Comparing base (
b04fb2e
) to head (49b3e3c
). Report is 269 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #1465 +/- ##
=======================================
Coverage 76.94% 76.94%
=======================================
Files 170 170
Lines 5668 5668
Branches 650 650
=======================================
Hits 4361 4361
Misses 1161 1161
Partials 146 146
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
So the changes I'm seeing make sense, though there are a number of logic changes I haven't been able to personally test. I'm kind of wondering what the criteria for updating is, since you left some of the old logic controls in components you updated. Is this intended to be ongoing?
So there were a few cases that I wasn't quite sure how we should handle. If I remember correctly, they were:
- using
*ngIf
format-error
and for certain, smallerspan
elements, as I wasn't quite sure if we want to use the new control flow there or if the normal inline approach is better - using
*ngFor
format-options
as I wasn't sure and didn't have time to check if using the new control flow works as expected
I think I'll use your comments to think again about whether it makes sense to apply the new control flow to all conditions and loops, or whether we should perhaps exclude some scenarios in the next planning to keep it simple. You are also welcome to use this PR or issue #1373 to provide your input on how we should use the new control flow. I think it is very important that we all align on this topic and ultimately include it in our development documentation.
And finally, this PR only serves as a starting point for the new control flow. This is mainly because this PR already contains a large number of changes and I didn't want to make it too big. Also, applying these changes takes some time, as the new control flow has a lot of room for major simplifications, especially since we can now use else if
and else
properly.
I really appreciate the effort you spent on the changes, but some components are really hard to test. Testing all of the modified changes will require a significant effort.
This week we've introduced storybook in the Capella Collaboration Manager. Storybook offers a nice UI and with the Chromatic integration stories will automatically scanned for visual changes. Due to mocking, the effort to implement stories low.
Therefore, we should have a story defined for the hard-to-test (e.g., authentication, logout) components to avoid some unintentional changes.
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code
Due to the amount of conflicts, I'll close the PR. Instead, we'll iteratively switch to the new format, together with the introduction of stories for the the affected components.