-
Notifications
You must be signed in to change notification settings - Fork 22
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
Create Account Confirmation Resend Button Thingy #609
Conversation
|
Name | Link |
---|---|
🔨 Latest commit | 91c3dfe |
Where is the button to resend the confirmation email? I can't find it. |
It's hard to see in the screenshot, but it's near the top left of the dashboard. And it's only there if you're logged in as an unconfirmed user. I don't have a "real" placement for it right now, as we don't have a design for it. |
f59fb2d
to
3a732a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mild comments, really. Nothing big.
Also we may need to update the styles eventually anyway, if UI comes back with a different design. Something to keep in mind.
@@ -10,6 +14,7 @@ export async function useGetUserDetail( | |||
const { onError, onOkay } = opts | |||
const responseData = ref<UserViewModel>({ | |||
id: '', | |||
role: 'Banned', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is Banned the default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty strings aren't allow based on the type of the property, so by default I assumed Banned
would be best since it would act like a user is not signed in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That feels like a hack. Is it possible to just use an empty object for the whole viewmodel instead? Maybe there's some other pattern that would work here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great example of where typescript hurts more than it helps. You should just be able to initialize it with a null value and then assume that it's not null if the fetch operation succeeds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the update on this? I believe there was a discussion in chat, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could just add an empty string to UserRole
, tbh. We would know in-code that we're in a logged out state if we see an empty string.
Obsoleted by #622. |
What
Frontend for resending the account confirmation email.
Link to Issue
Closes #592
Acceptance
Steps for testing
Any steps needed to test the feature (if applicable)
Images
Temp UI for now. Personally don't even like having a modal here; I'd prefer a toast: