canonical / identity-platform-login-ui

Login UI for the Canonical identity broker and identity provider solution
Apache License 2.0
10 stars 6 forks source link

Use new design and add user flows as dummy pages WD-8469 #216

Closed edlerd closed 8 months ago

edlerd commented 8 months ago
codecov[bot] commented 8 months ago

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (0f7031f) 76.55% compared to head (88bb621) 75.60%. Report is 118 commits behind head on main.

Files Patch % Lines
internal/monitoring/noop.go 0.00% 11 Missing :warning:
pkg/kratos/handlers.go 83.33% 3 Missing and 1 partial :warning:
internal/logging/noop.go 0.00% 2 Missing :warning:
pkg/kratos/service.go 94.44% 1 Missing and 1 partial :warning:
pkg/status/service.go 33.33% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #216 +/- ## ========================================== - Coverage 76.55% 75.60% -0.95% ========================================== Files 9 11 +2 Lines 627 537 -90 ========================================== - Hits 480 406 -74 + Misses 134 116 -18 - Partials 13 15 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

edlerd commented 8 months ago

it is quite a lot of code in just one commit, any chance to split it into smaller ones?

It's not that many code changes, most of the lines are due to dependency updates in package-lock.json, which are a prerequisite for other changes. So factoring them out into individual PRs is painful, because they would rely on each other.

Edit: I changed the structure of the PR, keeping the changes, but breaking them into a bit smaller commits.

shipperizer commented 8 months ago

@edlerd looks good to me afaict, what is the policy for frontend tests?how do we catch broken UI?

lukasSerelis commented 8 months ago

Looks good. MFA options obviously missing but I assume this is out of scope at this stage