-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
🪟 🎉 Add username editing #14242
🪟 🎉 Add username editing #14242
Changes from 3 commits
90c7ead
34fd3cb
6227682
680daaa
86afdc7
f0eedf2
0f0558d
a25799d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,62 +1,25 @@ | ||
import { Field, FieldProps, Form, Formik } from "formik"; | ||
import React from "react"; | ||
import { FormattedMessage, useIntl } from "react-intl"; | ||
import { FormattedMessage } from "react-intl"; | ||
import styled from "styled-components"; | ||
|
||
import { LabeledInput, LoadingButton } from "components"; | ||
import { LoadingButton } from "components"; | ||
|
||
import { useAuthService, useCurrentUser } from "packages/cloud/services/auth/AuthService"; | ||
import { RowFieldItem } from "packages/cloud/views/auth/components/FormComponents"; | ||
import { Content, SettingsCard } from "pages/SettingsPage/pages/SettingsComponents"; | ||
import { useAuthService } from "packages/cloud/services/auth/AuthService"; | ||
import { SettingsCard } from "pages/SettingsPage/pages/SettingsComponents"; | ||
|
||
import { EmailSection, PasswordSection } from "./components"; | ||
import { EmailSection, PasswordSection, NameSection } from "./components"; | ||
|
||
const Header = styled.div` | ||
display: flex; | ||
justify-content: space-between; | ||
`; | ||
|
||
const AccountSettingsView: React.FC = () => { | ||
const { formatMessage } = useIntl(); | ||
const { logout } = useAuthService(); | ||
const user = useCurrentUser(); | ||
|
||
return ( | ||
<> | ||
<SettingsCard title={<FormattedMessage id="settings.account" />}> | ||
<Content> | ||
<Formik | ||
initialValues={{ | ||
name: user.name, | ||
}} | ||
onSubmit={() => { | ||
throw new Error("Not implemented"); | ||
}} | ||
> | ||
{() => ( | ||
<Form> | ||
<RowFieldItem> | ||
<Field name="name"> | ||
{({ field, meta }: FieldProps<string>) => ( | ||
<LabeledInput | ||
{...field} | ||
label={<FormattedMessage id="settings.accountSettings.fullName" />} | ||
disabled | ||
placeholder={formatMessage({ | ||
id: "settings.accountSettings.fullName.placeholder", | ||
})} | ||
type="text" | ||
error={!!meta.error && meta.touched} | ||
message={meta.touched && meta.error && formatMessage({ id: meta.error })} | ||
/> | ||
)} | ||
</Field> | ||
</RowFieldItem> | ||
</Form> | ||
)} | ||
</Formik> | ||
</Content> | ||
</SettingsCard> | ||
<NameSection /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes it so much cleaner, thanks for extracting 🎉 |
||
<EmailSection /> | ||
<PasswordSection /> | ||
<SettingsCard | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
import { Field, FieldProps, Form, Formik } from "formik"; | ||
import React from "react"; | ||
import { FormattedMessage, useIntl } from "react-intl"; | ||
|
||
import { LoadingButton } from "components"; | ||
import { LabeledInput } from "components/LabeledInput"; | ||
|
||
import { useCurrentUser } from "packages/cloud/services/auth/AuthService"; | ||
import { RowFieldItem } from "packages/cloud/views/auth/components/FormComponents"; | ||
import FeedbackBlock from "pages/SettingsPage/components/FeedbackBlock"; | ||
import { Content, SettingsCard } from "pages/SettingsPage/pages/SettingsComponents"; | ||
|
||
import { useChangeName } from "./hooks"; | ||
edmundito marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
export const NameSection: React.FC = () => { | ||
const { formatMessage } = useIntl(); | ||
const user = useCurrentUser(); | ||
const { changeName, successMessage, errorMessage } = useChangeName(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great use of a hook to keep the component clean. |
||
|
||
return ( | ||
<SettingsCard title={<FormattedMessage id="settings.account" />}> | ||
<Content> | ||
<Formik | ||
initialValues={{ | ||
name: user.name, | ||
}} | ||
onSubmit={changeName} | ||
edmundito marked this conversation as resolved.
Show resolved
Hide resolved
|
||
> | ||
{({ isSubmitting }) => ( | ||
<Form> | ||
<RowFieldItem> | ||
<Field name="name"> | ||
{({ field, meta }: FieldProps<string>) => ( | ||
<LabeledInput | ||
{...field} | ||
label={<FormattedMessage id="settings.accountSettings.name" />} | ||
placeholder={formatMessage({ | ||
id: "settings.accountSettings.name.placeholder", | ||
})} | ||
type="text" | ||
error={!!meta.error && meta.touched} | ||
message={meta.touched && meta.error && formatMessage({ id: meta.error })} | ||
/> | ||
)} | ||
</Field> | ||
</RowFieldItem> | ||
<LoadingButton type="submit" isLoading={isSubmitting}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be disabled if the form is not valid |
||
<FormattedMessage id="settings.accountSettings.updateName" /> | ||
</LoadingButton> | ||
<FeedbackBlock errorMessage={errorMessage} successMessage={successMessage} /> | ||
</Form> | ||
)} | ||
</Formik> | ||
</Content> | ||
</SettingsCard> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { useChangeName } from "./useName"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import { FormikHelpers } from "formik/dist/types"; | ||
import { useState } from "react"; | ||
import { useIntl } from "react-intl"; | ||
|
||
import { useAuthService } from "packages/cloud/services/auth/AuthService"; | ||
|
||
import { FormValues } from "../types"; | ||
|
||
type UseNameHook = () => { | ||
successMessage: string; | ||
errorMessage: string; | ||
changeName: (values: FormValues, { setSubmitting, setFieldValue }: FormikHelpers<FormValues>) => void; | ||
}; | ||
|
||
export const useChangeName: UseNameHook = () => { | ||
const { updateName } = useAuthService(); | ||
const { formatMessage } = useIntl(); | ||
const [successMessage, setSuccessMessage] = useState<string>(""); | ||
const [errorMessage, setErrorMessage] = useState<string>(""); | ||
|
||
const changeName = async (values: FormValues, { setSubmitting }: FormikHelpers<FormValues>) => { | ||
setSubmitting(true); | ||
|
||
setSuccessMessage(""); | ||
setErrorMessage(""); | ||
|
||
try { | ||
await updateName(values.name); | ||
|
||
setSuccessMessage( | ||
formatMessage({ | ||
id: "settings.accountSettings.updateNameSuccess", | ||
}) | ||
); | ||
} catch (err) { | ||
switch (err.code) { | ||
default: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This switch does not seem necessary |
||
setErrorMessage( | ||
formatMessage({ | ||
id: "settings.accountSettings.updateNameError", | ||
}) + JSON.stringify(err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This error may be confusing to users. It may be better to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this logic is copied from PasswordSection. not sure, how the error should be approached here |
||
); | ||
} | ||
} | ||
|
||
setSubmitting(false); | ||
}; | ||
|
||
return { successMessage, errorMessage, changeName }; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { NameSection } from "./NameSection"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export interface FormValues { | ||
name: string; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
export * from "./EmailSection"; | ||
export * from "./PasswordSection"; | ||
export * from "./NameSection"; |
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.
As mentioned in a separate comment, this should never happen if we check that the current user exists first.
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.
I had this error, so that was only workaround here