feat: user experience improvements and e2e test coverage #321
Conversation
aeneasr
force-pushed
the
improve-ux-2
branch
3 times, most recently
from
190e6df to
7fa78d2
Compare
This patch resolves several issues: - Adds test IDs to components used by e2e selectors - Transports the return URL across flows (login/registration, recovery)
aeneasr
force-pushed
the
improve-ux-2
branch
3 times, most recently
from
1f21254 to
8deedd6
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #321 +/- ##
==========================================
+ Coverage 42.43% 42.80% +0.37%
==========================================
Files 136 138 +2
Lines 2008 2065 +57
Branches 288 303 +15
==========================================
+ Hits 852 884 +32
- Misses 1149 1172 +23
- Partials 7 9 +2
|
packages/elements-react/src/theme/default/components/card/current-identifier-button.tsx
Show resolved
Hide resolved
|
Also, this change makes the init flow helper available to consumers: #326 wdyt about that? |
|
The PR here is working for all flows, #326 is not. I recommend just merging this PR as it's been open long enough, and really doesn't change a lot of functionality except for fixing all the holes with e2e tests. #326 did not run against the cloud e2e tests i've been working on, and I don't have enough time to spend to rebase it all. So let's get this one here through, and then we can still apply the missing pieces to #326 and merge those improvements. |
| // Login has a special case where we only have one method. Here, we | ||
| // do not want to display the chooser. | ||
| const authMethods = nodesToAuthMethodGroups(flow.flow.ui.nodes) | ||
| if (authMethods.length === 1) { | ||
| return { current: "method_active", method: authMethods[0] } | ||
| } |
There was a problem hiding this comment.
This leads to this screen being shown if code is the only option for the user.
And in that case, we either need to auto submit the flow to then immediately show the code input, or do show the method selector so that the user can trigger the code sending.
There was a problem hiding this comment.
Fair point! I think auto-submit is the way to go. Ideally, this would be solved in kratos' identifier first
There was a problem hiding this comment.
Yea, actually I don't think we should work around this in Elements and just fix it in Kratos. So for now, let's show the method selector and fix it properly later in Kratos.
There was a problem hiding this comment.
But only for the code method please, not for password etc.
| flow: { request_url?: string; return_to?: string }, | ||
| fallback: string, | ||
| ) { | ||
| return flow.request_url || appendReturnTo(fallback, flow.return_to) |
There was a problem hiding this comment.
Why does this use the request_url? This URL is the callback URL for OIDC in account linking flows.
E.g. the user clicks on the "back button" in the flow, and is redirected back to the OIDC callback which leads to 404 (or similar).
There was a problem hiding this comment.
It‘s needed for all other flows and probably needs a fix in kratos account linking
There was a problem hiding this comment.
It's needed to restart any flow (back button functionality, timeouts, flow errors, ...) and properly set return to urls, requested aal, refresh, and other query parameters
This patch resolves several issues:
Related Issue or Design Document
Checklist
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact [email protected]) from the maintainers to push the changes.
Further comments