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.

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

chore: More migration to new Angular control flow #1804

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zusorio
Copy link
Member

@zusorio zusorio commented Sep 16, 2024

No description provided.

Copy link

codecov bot commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.60%. Comparing base (69159ef) to head (0910006).
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1804   +/-   ##
=======================================
  Coverage   84.60%   84.60%           
=======================================
  Files         203      203           
  Lines        6691     6691           
  Branches      742      742           
=======================================
  Hits         5661     5661           
  Misses        866      866           
  Partials      164      164           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MoritzWeber0
Copy link
Member

Hey @zusorio, great that you're looking into the migration of the control flow. We decided some time ago in the team to do it iteratively and to add stories for each touched component. So make sure that all components moved to the new control flow have stories if they make sense. If you spot CSS that can be moved to Tailwind, that's also a good place to do it. But as far as I can see, you're already on the right track :)

@zusorio zusorio marked this pull request as ready for review September 23, 2024 10:14
Copy link

github-actions bot commented Sep 23, 2024

A Storybook preview is available for commit b81e2d9.
➡️ View Storybook
➡️ View Chromatic build

This comment has been minimized.

This comment has been minimized.

Copy link
Member

@MoritzWeber0 MoritzWeber0 left a comment

Choose a reason for hiding this comment

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

A bit thanks for the migration and the stories 🫶
I have some smaller comments, in some cases I'd also add additional stories for specific cases.

@@ -0,0 +1,18 @@
/*
Copy link
Member

@MoritzWeber0 MoritzWeber0 Sep 23, 2024

Choose a reason for hiding this comment

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

A few more stories can be added to cover the diverse features of the component:

  1. Cover all steps of the mat-stepper (this can be done using the Storybook play function)
  2. Cover different model creation steps (update: only cover the last step where you can click "Finish")

frontend/src/storybook/monitoring.ts Outdated Show resolved Hide resolved

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link

sonarcloud bot commented Oct 8, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
6.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

Copy link

github-actions bot commented Oct 8, 2024

This report was generated by comparing b81e2d9 with 69159ef.
If you would like to check difference, please check here.

new items

ArtifactName: reg

item count
pass 318
change 0
new 22
delete 0
📝 Report

New Items

Model Components_Init Model_General_desktop.png

NewItem

Model Components_Init Model_General_mobile.png

NewItem

Project Components _ Audit Log_General_desktop.png

NewItem

Project Components _ Audit Log_General_mobile.png

NewItem

Project Components _ Audit Log_Loading_desktop.png

NewItem

Project Components _ Audit Log_Loading_mobile.png

NewItem

Project Components _ Create Project_Add Model_desktop.png

NewItem

Project Components _ Create Project_Add Model_mobile.png

NewItem

Project Components _ Create Project_Add Team Members_desktop.png

NewItem

Project Components _ Create Project_Add Team Members_mobile.png

NewItem

Project Components _ Create Project_Create Project_desktop.png

NewItem

Project Components _ Create Project_Create Project_mobile.png

NewItem

Project Components_Project Details_Edit Project Metadata_General_desktop.png

NewItem

Project Components_Project Details_Edit Project Metadata_General_mobile.png

NewItem

Project Components_Project Details_Edit Project Metadata_With Title_desktop.png

NewItem

Project Components_Project Details_Edit Project Metadata_With Title_mobile.png

NewItem

Settings Components_Pipelines Overview_Bad Health_desktop.png

NewItem

Settings Components_Pipelines Overview_Bad Health_mobile.png

NewItem

Settings Components_Pipelines Overview_Good Health_desktop.png

NewItem

Settings Components_Pipelines Overview_Good Health_mobile.png

NewItem

Settings Components_Pipelines Overview_Loading_desktop.png

NewItem

Settings Components_Pipelines Overview_Loading_mobile.png

NewItem

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.

2 participants