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

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

Merged
merged 4 commits into from
Jan 30, 2024

Conversation

edlerd
Copy link
Contributor

@edlerd edlerd commented Jan 26, 2024

  • adjusted to new design
  • added dummy pages for register, reset, device and setup

@edlerd edlerd requested a review from a team as a code owner January 26, 2024 12:05
@edlerd edlerd marked this pull request as draft January 26, 2024 12:05
@edlerd edlerd force-pushed the add-user-flows-and-new-design branch from a803eb2 to 95e5f77 Compare January 26, 2024 12:20
Copy link

codecov bot commented Jan 26, 2024

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 ⚠️
pkg/kratos/handlers.go 83.33% 3 Missing and 1 partial ⚠️
internal/logging/noop.go 0.00% 2 Missing ⚠️
pkg/kratos/service.go 94.44% 1 Missing and 1 partial ⚠️
pkg/status/service.go 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            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     

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

@edlerd edlerd force-pushed the add-user-flows-and-new-design branch 2 times, most recently from ecc0dee to f8f98c1 Compare January 29, 2024 18:54
@edlerd edlerd changed the title Use new design and add user flows as dummy pages Use new design and add user flows as dummy pages WD-8469 Jan 29, 2024
@edlerd edlerd marked this pull request as ready for review January 29, 2024 20:37
Copy link
Contributor

@shipperizer shipperizer left a comment

Choose a reason for hiding this comment

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

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

rockcraft.yaml Show resolved Hide resolved
@edlerd
Copy link
Contributor Author

edlerd commented Jan 29, 2024

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.

ui/pages/login.tsx Outdated Show resolved Hide resolved
@edlerd edlerd force-pushed the add-user-flows-and-new-design branch from f8f98c1 to 6e1b5fd Compare January 30, 2024 09:13
Copy link
Contributor

@nsklikas nsklikas left a comment

Choose a reason for hiding this comment

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

LGTM other than the comment I made on the login page. One thing I don't understand is what the plan is for these registration pages, the fields are going to come from Kratos and I would expect that we are going to ask for more attributes rather than just username and password. We can iterate on this further when we do the work on the backend so I have no problem with merging this once the issue on the login page is fixed.

Here are some screen shots on what the new pages look like:
Login Page:
image

OIDC Error:
image

Register Secure:
image

Register Complete:
image

Register Email:
image

Register Password:
image

Register Secure:
image

Reset Check:
image

Reset Complete:
image

Reset Email:
image

Reset Password:
image

Setup Complete:
image

Setup Password:
image

Setup Secure:
image

@shipperizer
Copy link
Contributor

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

Copy link
Contributor

@nsklikas nsklikas left a comment

Choose a reason for hiding this comment

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

One more minor thing and I think we can merge it

ui/components/NodeInputSubmit.tsx Outdated Show resolved Hide resolved
ui/public/logos/Okta.svg Outdated Show resolved Hide resolved
@edlerd edlerd force-pushed the add-user-flows-and-new-design branch from c4e1ff8 to 88bb621 Compare January 30, 2024 10:35
Copy link
Contributor

@nsklikas nsklikas left a comment

Choose a reason for hiding this comment

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

LGTM

@nsklikas nsklikas merged commit a4a9d49 into canonical:main Jan 30, 2024
6 of 8 checks passed
@lukasSerelis
Copy link

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

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.

4 participants