From a0c8424735564f10f7775f28c52650118c0bf0a1 Mon Sep 17 00:00:00 2001 From: Artem Ruts <1319181+artemruts@users.noreply.github.com> Date: Fri, 13 Nov 2020 11:10:11 -0500 Subject: [PATCH 01/22] cleanup before mergin main --- .../user/settings/emails/AddUserEmailForm.tsx | 4 +- .../emails/SetUserPrimaryEmailForm.tsx | 98 +++++++++++++++ .../src/user/settings/emails/UserEmail.tsx | 98 +++++++++++++++ .../settings/emails/UserEmailSettings.tsx | 114 ++++++++++++++++++ client/web/src/user/settings/routes.tsx | 2 +- 5 files changed, 313 insertions(+), 3 deletions(-) create mode 100644 client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx create mode 100644 client/web/src/user/settings/emails/UserEmail.tsx create mode 100644 client/web/src/user/settings/emails/UserEmailSettings.tsx diff --git a/client/web/src/user/settings/emails/AddUserEmailForm.tsx b/client/web/src/user/settings/emails/AddUserEmailForm.tsx index 5d9bc2650846..145cf52cd031 100644 --- a/client/web/src/user/settings/emails/AddUserEmailForm.tsx +++ b/client/web/src/user/settings/emails/AddUserEmailForm.tsx @@ -15,7 +15,7 @@ interface Props { user: GQL.ID /** Called after successfully adding an email to the user. */ - onDidAdd: () => void + onDidAdd: (email: string) => void className?: string history: H.History @@ -41,7 +41,7 @@ export class AddUserEmailForm extends React.PureComponent { merge( of>({ error: undefined }), this.addUserEmail(this.state.email).pipe( - tap(() => this.props.onDidAdd()), + tap(() => this.props.onDidAdd(this.state.email)), map(() => ({ error: null, email: '' })), catchError(error => [{ error, email: this.state.email }]) ) diff --git a/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx new file mode 100644 index 000000000000..d1b70932a56b --- /dev/null +++ b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx @@ -0,0 +1,98 @@ +/* eslint-disable react/jsx-no-bind */ +import React, { useState, FunctionComponent } from 'react' +import * as H from 'history' +import { Form } from '../../../../../branded/src/components/Form' +import { IUserEmail } from '../../../../../shared/src/graphql/schema' +import { mutateGraphQL } from '../../../backend/graphql' +import { gql } from '../../../../../shared/src/graphql/graphql' +import { createAggregateError } from '../../../../../shared/src/util/errors' +import { eventLogger } from '../../../tracking/eventLogger' +import { ErrorAlert } from '../../../components/alerts' + +interface Props { + userId: string + emails: IUserEmail[] + onDidSet: (email: string) => void + history: H.History + className?: string +} + +interface UserEmailState { + loading: boolean + errorDescription: Error | null +} + +export const SetUserPrimaryEmailForm: FunctionComponent = ({ userId, emails, onDidSet, className, history }) => { + const [primaryEmail, setPrimaryEmail] = useState('') + const [status, setStatus] = useState({ loading: false, errorDescription: null }) + + const onPrimaryEmailSelect: React.ChangeEventHandler = event => + setPrimaryEmail(event.target.value) + + const onSubmit: React.FormEventHandler = async event => { + event.preventDefault() + + const { data, errors } = mutateGraphQL( + gql` + mutation SetUserEmailPrimary($user: ID!, $email: String!) { + setUserEmailPrimary(user: $user, email: $email) { + alwaysNil + } + } + `, + { user: userId, email: primaryEmail } + ).toPromise() + + // TODO: check this + if (!data || (errors && errors.length > 0)) { + const aggregateError = createAggregateError(errors) + setStatus({ loading: false, errorDescription: aggregateError }) + throw aggregateError + } + + setStatus({ ...status, loading: false }) + + if (onDidSet) { + onDidSet(primaryEmail) + } + eventLogger.log('UserEmailAddressSetAsPrimary') + } + + return ( +
+

Set primary email address

+
+ + {' '} + +
+ {/* TODO: check this */} + {status.errorDescription && ( + + )} +
+ ) +} diff --git a/client/web/src/user/settings/emails/UserEmail.tsx b/client/web/src/user/settings/emails/UserEmail.tsx new file mode 100644 index 000000000000..78ec76435086 --- /dev/null +++ b/client/web/src/user/settings/emails/UserEmail.tsx @@ -0,0 +1,98 @@ +import React, { useState, FunctionComponent } from 'react' +import classNames from 'classnames' +import * as H from 'history' + +import { mutateGraphQL } from '../../../backend/graphql' +import { IUserEmail } from '../../../../../shared/src/graphql/schema' +import { gql } from '../../../../../shared/src/graphql/graphql' + +import { eventLogger } from '../../../tracking/eventLogger' +import { createAggregateError } from '../../../../../shared/src/util/errors' +import { ErrorAlert } from '../../../components/alerts' + +interface Props { + user: string + email: IUserEmail + history: H.History + onDidRemove?: (email: string) => void +} + +interface UserEmailState { + loading: boolean + errorDescription: Error | null +} + +export const UserEmail: FunctionComponent = ({ + user, + email: { email, isPrimary, verified, verificationPending }, + onDidRemove, + history, +}) => { + const [status, setStatus] = useState({ loading: false, errorDescription: null }) + + let verifiedFragment: React.ReactFragment + if (verified) { + verifiedFragment = Verified + } else if (verificationPending) { + verifiedFragment = Verification pending + } else { + verifiedFragment = Not verified + } + + const removeUserEmail = async (): Promise => { + if (!window.confirm(`Remove the email address ${email}?`)) { + return + } + + setStatus({ ...status, loading: true }) + + const { data, errors } = await mutateGraphQL( + gql` + mutation RemoveUserEmail($user: ID!, $email: String!) { + removeUserEmail(user: $user, email: $email) { + alwaysNil + } + } + `, + { user, email } + ).toPromise() + + // TODO: check this + if (!data || (errors && errors.length > 0)) { + const aggregateError = createAggregateError(errors) + setStatus({ loading: false, errorDescription: aggregateError }) + throw aggregateError + } + + setStatus({ ...status, loading: false }) + + if (onDidRemove) { + onDidRemove(email) + } + eventLogger.log('UserEmailAddressDeleted') + } + + const removeLinkClasses = classNames({ + btn: true, + 'text-danger': !status.loading, + 'text-muted': status.loading, + }) + + return ( +
+
+ {email}  {verifiedFragment}    + {isPrimary && Primary} +
+ {verified && ( + {} : removeUserEmail}> + Remove + + )} + {/* TODO: check this */} + {status.errorDescription && ( + + )} +
+ ) +} diff --git a/client/web/src/user/settings/emails/UserEmailSettings.tsx b/client/web/src/user/settings/emails/UserEmailSettings.tsx new file mode 100644 index 000000000000..a8cf613c30a0 --- /dev/null +++ b/client/web/src/user/settings/emails/UserEmailSettings.tsx @@ -0,0 +1,114 @@ +/* eslint-disable react/jsx-no-bind */ +import React, { FunctionComponent, useEffect, useState } from 'react' +import { RouteComponentProps } from 'react-router' +import * as H from 'history' + +import { queryGraphQL } from '../../../backend/graphql' +import { UserAreaUserFields } from '../../../graphql-operations' +import { gql } from '../../../../../shared/src/graphql/graphql' +import * as GQL from '../../../../../shared/src/graphql/schema' +import { createAggregateError } from '../../../../../shared/src/util/errors' + +import { PageTitle } from '../../../components/PageTitle' +import { UserEmail } from './UserEmail' +import { AddUserEmailForm } from './AddUserEmailForm' +import { SetUserPrimaryEmailForm } from './SetUserPrimaryEmailForm' + +interface Props extends RouteComponentProps<{}> { + user: UserAreaUserFields + history: H.History +} + +export const UserEmailSettings: FunctionComponent = ({ user, history }) => { + const [emails, setEmails] = useState([]) + + const onEmailAdd = (email: string): void => { + const newEmail = { + email, + isPrimary: false, + verified: false, + verificationPending: true, + } + + // TODO: pick types instead of IUserEmail + setEmails([...emails, newEmail]) + } + + const onEmailRemove = (deletedEmail: string): void => { + // seems like page needs tp be re-fetched or maybe use optimistc? + setEmails(emails.filter(({ email }) => email !== deletedEmail)) + } + + const onPrimaryEmailSet = (email: string): void => { + console.log(email) + // find current primary email + // set as non-primary + // find the one we just set and make it a primary + // setEmails + } + + useEffect(() => { + const fetchUserEmails = async (): Promise => { + // TODO + const { data, errors } = await queryGraphQL( + gql` + query UserEmails($user: ID!) { + node(id: $user) { + ... on User { + emails { + email + isPrimary + verified + verificationPending + viewerCanManuallyVerify + } + } + } + } + `, + { user: user.id } + ).toPromise() + + if (errors || !data || !data?.node) { + // TODO: why? + throw createAggregateError(errors) + } + + const userResult = data.node as GQL.IUser + + setEmails(userResult.emails) + } + + // TODO + fetchUserEmails().catch(error => { + throw error + }) + }, [user.id, setEmails]) + + return ( +
+ +

Emails

+
+ {/* TODO: Fix this class */} +
    + {emails.map(email => ( + // TODO: fix this +
  • + +
    • + ))} +
+
+ +
+ +
+ ) +} diff --git a/client/web/src/user/settings/routes.tsx b/client/web/src/user/settings/routes.tsx index 41bb012dec2c..4f5622e7566c 100644 --- a/client/web/src/user/settings/routes.tsx +++ b/client/web/src/user/settings/routes.tsx @@ -55,7 +55,7 @@ export const userSettingsAreaRoutes: readonly UserSettingsAreaRoute[] = [ { path: '/emails', exact: true, - render: lazyComponent(() => import('./emails/UserSettingsEmailsPage'), 'UserSettingsEmailsPage'), + render: lazyComponent(() => import('./emails/UserEmailSettings'), 'UserEmailSettings'), }, { path: '/tokens', From 529922842e4f62603310411e91721811583b09ae Mon Sep 17 00:00:00 2001 From: Artem Ruts <1319181+artemruts@users.noreply.github.com> Date: Fri, 13 Nov 2020 22:57:57 -0500 Subject: [PATCH 02/22] Beta version --- .../emails/SetUserPrimaryEmailForm.tsx | 89 ++++++++----- .../src/user/settings/emails/UserEmail.tsx | 96 +++++++++----- .../settings/emails/UserEmailSettings.tsx | 123 +++++++++++------- client/web/src/user/settings/routes.tsx | 5 + 4 files changed, 204 insertions(+), 109 deletions(-) diff --git a/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx index d1b70932a56b..d020067de875 100644 --- a/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx +++ b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx @@ -1,5 +1,5 @@ /* eslint-disable react/jsx-no-bind */ -import React, { useState, FunctionComponent } from 'react' +import React, { useState, FunctionComponent, useEffect } from 'react' import * as H from 'history' import { Form } from '../../../../../branded/src/components/Form' import { IUserEmail } from '../../../../../shared/src/graphql/schema' @@ -10,7 +10,7 @@ import { eventLogger } from '../../../tracking/eventLogger' import { ErrorAlert } from '../../../components/alerts' interface Props { - userId: string + user: string emails: IUserEmail[] onDidSet: (email: string) => void history: H.History @@ -22,17 +22,53 @@ interface UserEmailState { errorDescription: Error | null } -export const SetUserPrimaryEmailForm: FunctionComponent = ({ userId, emails, onDidSet, className, history }) => { - const [primaryEmail, setPrimaryEmail] = useState('') +export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails, onDidSet, className, history }) => { + const [primaryEmail, setPrimaryEmail] = useState('') + const [disabled, setDisabled] = useState(false) + const [emailOptions, setEmailOptions] = useState([]) const [status, setStatus] = useState({ loading: false, errorDescription: null }) + useEffect(() => { + const possibleEmails = [] + let placeholderPrimaryEmail = '' + + for (const email of emails) { + // collect possible primary emails + if (!email.isPrimary && email.verified) { + possibleEmails.push(email.email) + } + + if (email.isPrimary) { + // there can be only one + placeholderPrimaryEmail = email.email + } + } + + const shouldDisable = possibleEmails.length === 0 + + let options + let currentPrimaryEmail + + if (possibleEmails.length !== 0) { + options = possibleEmails + currentPrimaryEmail = possibleEmails[0] + } else { + options = [placeholderPrimaryEmail] + currentPrimaryEmail = placeholderPrimaryEmail + } + + setDisabled(shouldDisable) + setEmailOptions(options) + setPrimaryEmail(currentPrimaryEmail) + }, [emails]) + const onPrimaryEmailSelect: React.ChangeEventHandler = event => setPrimaryEmail(event.target.value) const onSubmit: React.FormEventHandler = async event => { event.preventDefault() - const { data, errors } = mutateGraphQL( + const { data, errors } = await mutateGraphQL( gql` mutation SetUserEmailPrimary($user: ID!, $email: String!) { setUserEmailPrimary(user: $user, email: $email) { @@ -40,22 +76,19 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ userId, emai } } `, - { user: userId, email: primaryEmail } + { user, email: primaryEmail } ).toPromise() - // TODO: check this if (!data || (errors && errors.length > 0)) { - const aggregateError = createAggregateError(errors) - setStatus({ loading: false, errorDescription: aggregateError }) - throw aggregateError - } - - setStatus({ ...status, loading: false }) + setStatus({ loading: false, errorDescription: createAggregateError(errors) }) + } else { + eventLogger.log('UserEmailAddressSetAsPrimary') + setStatus({ ...status, loading: false }) - if (onDidSet) { - onDidSet(primaryEmail) + if (onDidSet) { + onDidSet(primaryEmail) + } } - eventLogger.log('UserEmailAddressSetAsPrimary') } return ( @@ -67,29 +100,23 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ userId, emai {' '} - - {/* TODO: check this */} {status.errorDescription && ( )} diff --git a/client/web/src/user/settings/emails/UserEmail.tsx b/client/web/src/user/settings/emails/UserEmail.tsx index 78ec76435086..2c7d97f41977 100644 --- a/client/web/src/user/settings/emails/UserEmail.tsx +++ b/client/web/src/user/settings/emails/UserEmail.tsx @@ -10,25 +10,33 @@ import { eventLogger } from '../../../tracking/eventLogger' import { createAggregateError } from '../../../../../shared/src/util/errors' import { ErrorAlert } from '../../../components/alerts' +interface VerificationUpdate { + email: string + verified: boolean +} + interface Props { user: string email: IUserEmail history: H.History + onDidRemove?: (email: string) => void + onEmailVerify?: (update: VerificationUpdate) => void } -interface UserEmailState { +interface LoadingState { loading: boolean errorDescription: Error | null } export const UserEmail: FunctionComponent = ({ user, - email: { email, isPrimary, verified, verificationPending }, + email: { email, isPrimary, verified, verificationPending, viewerCanManuallyVerify }, onDidRemove, + onEmailVerify, history, }) => { - const [status, setStatus] = useState({ loading: false, errorDescription: null }) + const [status, setStatus] = useState({ loading: false, errorDescription: null }) let verifiedFragment: React.ReactFragment if (verified) { @@ -39,7 +47,7 @@ export const UserEmail: FunctionComponent = ({ verifiedFragment = Not verified } - const removeUserEmail = async (): Promise => { + const removeEmail = async (): Promise => { if (!window.confirm(`Remove the email address ${email}?`)) { return } @@ -57,42 +65,72 @@ export const UserEmail: FunctionComponent = ({ { user, email } ).toPromise() - // TODO: check this if (!data || (errors && errors.length > 0)) { - const aggregateError = createAggregateError(errors) - setStatus({ loading: false, errorDescription: aggregateError }) - throw aggregateError + setStatus({ loading: false, errorDescription: createAggregateError(errors) }) + } else { + setStatus({ ...status, loading: false }) + eventLogger.log('UserEmailAddressDeleted') + + if (onDidRemove) { + onDidRemove(email) + } } + } - setStatus({ ...status, loading: false }) + const updateEmailVerification = async (verified: boolean): Promise => { + setStatus({ ...status, loading: true }) - if (onDidRemove) { - onDidRemove(email) + const { data, errors } = await mutateGraphQL( + gql` + mutation SetUserEmailVerified($user: ID!, $email: String!, $verified: Boolean!) { + setUserEmailVerified(user: $user, email: $email, verified: $verified) { + alwaysNil + } + } + `, + { user, email, verified } + ).toPromise() + + if (!data || (errors && errors.length > 0)) { + setStatus({ loading: false, errorDescription: createAggregateError(errors) }) + } else { + setStatus({ ...status, loading: false }) + + if (verified) { + eventLogger.log('UserEmailAddressMarkedVerified') + } else { + eventLogger.log('UserEmailAddressMarkedUnverified') + } + + if (onEmailVerify) { + onEmailVerify({ email, verified }) + } } - eventLogger.log('UserEmailAddressDeleted') } - const removeLinkClasses = classNames({ - btn: true, - 'text-danger': !status.loading, - 'text-muted': status.loading, - }) - return ( -
-
- {email}  {verifiedFragment}    - {isPrimary && Primary} + <> +
+
+ {email}  {verifiedFragment}    + {isPrimary && Primary} +
+
+ {viewerCanManuallyVerify ? ( + updateEmailVerification(!verified)}> + {verified ? 'Mark as unverified' : 'Mark as verified'} + + ) : null} + {!isPrimary ? ( + {}}> + Remove + + ) : null} +
- {verified && ( - {} : removeUserEmail}> - Remove - - )} - {/* TODO: check this */} {status.errorDescription && ( )} -
+ ) } diff --git a/client/web/src/user/settings/emails/UserEmailSettings.tsx b/client/web/src/user/settings/emails/UserEmailSettings.tsx index a8cf613c30a0..1e4aefc5bab4 100644 --- a/client/web/src/user/settings/emails/UserEmailSettings.tsx +++ b/client/web/src/user/settings/emails/UserEmailSettings.tsx @@ -1,5 +1,5 @@ /* eslint-disable react/jsx-no-bind */ -import React, { FunctionComponent, useEffect, useState } from 'react' +import React, { FunctionComponent, useEffect, useState, useCallback } from 'react' import { RouteComponentProps } from 'react-router' import * as H from 'history' @@ -9,6 +9,8 @@ import { gql } from '../../../../../shared/src/graphql/graphql' import * as GQL from '../../../../../shared/src/graphql/schema' import { createAggregateError } from '../../../../../shared/src/util/errors' +import { eventLogger } from '../../../tracking/eventLogger' +import { ErrorAlert } from '../../../components/alerts' import { PageTitle } from '../../../components/PageTitle' import { UserEmail } from './UserEmail' import { AddUserEmailForm } from './AddUserEmailForm' @@ -19,92 +21,115 @@ interface Props extends RouteComponentProps<{}> { history: H.History } +interface LoadingState { + loading: boolean + errorDescription: Error | null +} + export const UserEmailSettings: FunctionComponent = ({ user, history }) => { const [emails, setEmails] = useState([]) + const [status, setStatus] = useState({ loading: false, errorDescription: null }) - const onEmailAdd = (email: string): void => { - const newEmail = { - email, - isPrimary: false, - verified: false, - verificationPending: true, - } + const updateNewPrimaryEmail = (updatedEmail: string): GQL.IUserEmail[] => + emails.map(email => { + if (email.isPrimary && email.email !== updatedEmail) { + email.isPrimary = false + } + + if (email.email === updatedEmail) { + email.isPrimary = true + } + + return email + }) + + const onEmailVerify = ({ email: verifiedEmail, verified }: { email: string; verified: boolean }): void => { + const updatedEmails = emails.map(email => { + if (email.email === verifiedEmail) { + email.verified = verified + } - // TODO: pick types instead of IUserEmail - setEmails([...emails, newEmail]) + return email + }) + + setEmails(updatedEmails) } const onEmailRemove = (deletedEmail: string): void => { - // seems like page needs tp be re-fetched or maybe use optimistc? setEmails(emails.filter(({ email }) => email !== deletedEmail)) } const onPrimaryEmailSet = (email: string): void => { - console.log(email) - // find current primary email - // set as non-primary - // find the one we just set and make it a primary - // setEmails + setEmails(updateNewPrimaryEmail(email)) } - useEffect(() => { - const fetchUserEmails = async (): Promise => { - // TODO - const { data, errors } = await queryGraphQL( - gql` - query UserEmails($user: ID!) { - node(id: $user) { - ... on User { - emails { - email - isPrimary - verified - verificationPending - viewerCanManuallyVerify - } + const fetchEmails = useCallback(async (): Promise => { + const { data, errors } = await queryGraphQL( + gql` + query UserEmails($user: ID!) { + node(id: $user) { + ... on User { + emails { + email + isPrimary + verified + verificationPending + viewerCanManuallyVerify } } } - `, - { user: user.id } - ).toPromise() - - if (errors || !data || !data?.node) { - // TODO: why? - throw createAggregateError(errors) - } + } + `, + { user: user.id } + ).toPromise() + if (!data || (errors && errors.length > 0)) { + setStatus({ loading: false, errorDescription: createAggregateError(errors) }) + } else { + setStatus({ errorDescription: null, loading: false }) const userResult = data.node as GQL.IUser - setEmails(userResult.emails) } + }, [user]) - // TODO - fetchUserEmails().catch(error => { - throw error - }) - }, [user.id, setEmails]) + useEffect(() => { + eventLogger.logViewEvent('UserSettingsEmails') + }, []) + + useEffect(() => { + console.log('EmailSettings') + // eslint-disable-next-line @typescript-eslint/no-floating-promises + fetchEmails() + }, [fetchEmails]) return (

Emails

+ {status.errorDescription && ( + + )}
{/* TODO: Fix this class */}
    {emails.map(email => ( - // TODO: fix this
  • - +
    • ))}
- +
import('./emails/UserEmailSettings'), 'UserEmailSettings'), }, + { + path: '/emails-o', + exact: true, + render: lazyComponent(() => import('./emails/UserSettingsEmailsPage'), 'UserSettingsEmailsPage'), + }, { path: '/tokens', exact: true, From 476fcefa33acab3cf45c6224201d69d9e2c54585 Mon Sep 17 00:00:00 2001 From: Artem Ruts <1319181+artemruts@users.noreply.github.com> Date: Fri, 13 Nov 2020 23:06:34 -0500 Subject: [PATCH 03/22] Remove unused imports --- client/web/src/user/settings/emails/UserEmail.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/client/web/src/user/settings/emails/UserEmail.tsx b/client/web/src/user/settings/emails/UserEmail.tsx index 2c7d97f41977..b575de8c34f1 100644 --- a/client/web/src/user/settings/emails/UserEmail.tsx +++ b/client/web/src/user/settings/emails/UserEmail.tsx @@ -1,5 +1,4 @@ import React, { useState, FunctionComponent } from 'react' -import classNames from 'classnames' import * as H from 'history' import { mutateGraphQL } from '../../../backend/graphql' From 91c420193a4290a20dc7134201a479802f4dbf61 Mon Sep 17 00:00:00 2001 From: Artem Ruts <1319181+artemruts@users.noreply.github.com> Date: Wed, 18 Nov 2020 00:34:43 -0500 Subject: [PATCH 04/22] Ready for comments --- .../user/settings/emails/AddUserEmailForm.tsx | 5 +- .../emails/SetUserPrimaryEmailForm.tsx | 39 +- .../src/user/settings/emails/UserEmail.tsx | 95 +++-- .../settings/emails/UserEmailSettings.tsx | 139 ------- .../emails/UserSettingsEmailsPage.tsx | 378 ++++++------------ client/web/src/user/settings/routes.tsx | 5 - 6 files changed, 225 insertions(+), 436 deletions(-) delete mode 100644 client/web/src/user/settings/emails/UserEmailSettings.tsx diff --git a/client/web/src/user/settings/emails/AddUserEmailForm.tsx b/client/web/src/user/settings/emails/AddUserEmailForm.tsx index 145cf52cd031..ec02543263c6 100644 --- a/client/web/src/user/settings/emails/AddUserEmailForm.tsx +++ b/client/web/src/user/settings/emails/AddUserEmailForm.tsx @@ -8,6 +8,7 @@ import { mutateGraphQL } from '../../../backend/graphql' import { Form } from '../../../../../branded/src/components/Form' import { eventLogger } from '../../../tracking/eventLogger' import { ErrorAlert } from '../../../components/alerts' +import { LoaderButton } from '../../../components/LoaderButton' import * as H from 'history' interface Props { @@ -83,9 +84,7 @@ export class AddUserEmailForm extends React.PureComponent { readOnly={loading} placeholder="Email" />{' '} - + {this.state.error && ( diff --git a/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx index d020067de875..e084166f6aa9 100644 --- a/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx +++ b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx @@ -1,12 +1,15 @@ /* eslint-disable react/jsx-no-bind */ import React, { useState, FunctionComponent, useEffect } from 'react' import * as H from 'history' -import { Form } from '../../../../../branded/src/components/Form' + import { IUserEmail } from '../../../../../shared/src/graphql/schema' import { mutateGraphQL } from '../../../backend/graphql' import { gql } from '../../../../../shared/src/graphql/graphql' import { createAggregateError } from '../../../../../shared/src/util/errors' import { eventLogger } from '../../../tracking/eventLogger' + +import { Form } from '../../../../../branded/src/components/Form' +import { LoaderButton } from '../../../components/LoaderButton' import { ErrorAlert } from '../../../components/alerts' interface Props { @@ -24,14 +27,21 @@ interface UserEmailState { export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails, onDidSet, className, history }) => { const [primaryEmail, setPrimaryEmail] = useState('') - const [disabled, setDisabled] = useState(false) const [emailOptions, setEmailOptions] = useState([]) const [status, setStatus] = useState({ loading: false, errorDescription: null }) + const [disabled, setDisabled] = useState(false) useEffect(() => { const possibleEmails = [] let placeholderPrimaryEmail = '' + /** + * Every time emails props changes, find: + * + * 1. all emails that can be set as primary (not primary and verified) + * 2. current primary email to be used as UI "placeholder" for disabled + * select + */ for (const email of emails) { // collect possible primary emails if (!email.isPrimary && email.verified) { @@ -47,19 +57,23 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails const shouldDisable = possibleEmails.length === 0 let options - let currentPrimaryEmail + let selectValue + /** + * If possible primary emails were found, use them as select's options + * and set the first email as select's value + */ if (possibleEmails.length !== 0) { options = possibleEmails - currentPrimaryEmail = possibleEmails[0] + selectValue = possibleEmails[0] } else { options = [placeholderPrimaryEmail] - currentPrimaryEmail = placeholderPrimaryEmail + selectValue = placeholderPrimaryEmail } setDisabled(shouldDisable) setEmailOptions(options) - setPrimaryEmail(currentPrimaryEmail) + setPrimaryEmail(selectValue) }, [emails]) const onPrimaryEmailSelect: React.ChangeEventHandler = event => @@ -67,6 +81,7 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails const onSubmit: React.FormEventHandler = async event => { event.preventDefault() + setStatus({ loading: true, errorDescription: null }) const { data, errors } = await mutateGraphQL( gql` @@ -83,7 +98,7 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails setStatus({ loading: false, errorDescription: createAggregateError(errors) }) } else { eventLogger.log('UserEmailAddressSetAsPrimary') - setStatus({ ...status, loading: false }) + setStatus({ loading: false, errorDescription: null }) if (onDidSet) { onDidSet(primaryEmail) @@ -113,9 +128,13 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails ))} {' '} - + {status.errorDescription && ( diff --git a/client/web/src/user/settings/emails/UserEmail.tsx b/client/web/src/user/settings/emails/UserEmail.tsx index b575de8c34f1..984deb3315aa 100644 --- a/client/web/src/user/settings/emails/UserEmail.tsx +++ b/client/web/src/user/settings/emails/UserEmail.tsx @@ -1,4 +1,5 @@ -import React, { useState, FunctionComponent } from 'react' +import React, { useState, useCallback, FunctionComponent } from 'react' +import { Modal } from 'reactstrap' import * as H from 'history' import { mutateGraphQL } from '../../../backend/graphql' @@ -8,6 +9,7 @@ import { gql } from '../../../../../shared/src/graphql/graphql' import { eventLogger } from '../../../tracking/eventLogger' import { createAggregateError } from '../../../../../shared/src/util/errors' import { ErrorAlert } from '../../../components/alerts' +import { LoaderButton } from '../../../components/LoaderButton' interface VerificationUpdate { email: string @@ -36,21 +38,11 @@ export const UserEmail: FunctionComponent = ({ history, }) => { const [status, setStatus] = useState({ loading: false, errorDescription: null }) + const [modal, setModal] = useState(false) - let verifiedFragment: React.ReactFragment - if (verified) { - verifiedFragment = Verified - } else if (verificationPending) { - verifiedFragment = Verification pending - } else { - verifiedFragment = Not verified - } - - const removeEmail = async (): Promise => { - if (!window.confirm(`Remove the email address ${email}?`)) { - return - } + const toggleModal = useCallback(() => setModal(!modal), [modal]) + const removeEmail = useCallback(async (): Promise => { setStatus({ ...status, loading: true }) const { data, errors } = await mutateGraphQL( @@ -74,7 +66,9 @@ export const UserEmail: FunctionComponent = ({ onDidRemove(email) } } - } + + setModal(false) + }, [email, status, user, onDidRemove]) const updateEmailVerification = async (verified: boolean): Promise => { setStatus({ ...status, loading: true }) @@ -107,29 +101,84 @@ export const UserEmail: FunctionComponent = ({ } } + let verifiedLinkFragment: React.ReactFragment + if (verified) { + verifiedLinkFragment = Verified + } else if (verificationPending) { + verifiedLinkFragment = Verification pending + } else { + verifiedLinkFragment = Not verified + } + + const removeLinkFragment: React.ReactFragment = isPrimary ? ( + + Remove + + ) : ( + + Remove + + ) + return ( <>
- {email}  {verifiedFragment}    + {email}  {verifiedLinkFragment}    {isPrimary && Primary}
- {viewerCanManuallyVerify ? ( + {viewerCanManuallyVerify && ( updateEmailVerification(!verified)}> {verified ? 'Mark as unverified' : 'Mark as verified'} - ) : null} - {!isPrimary ? ( - {}}> - Remove - - ) : null} + )} + {removeLinkFragment}
{status.errorDescription && ( )} + {modal && ( + +
+

Remove email

+ +
+
+

+ Remove the email address {email}? +

+
+
+ + +
+ + )} ) } diff --git a/client/web/src/user/settings/emails/UserEmailSettings.tsx b/client/web/src/user/settings/emails/UserEmailSettings.tsx deleted file mode 100644 index 1e4aefc5bab4..000000000000 --- a/client/web/src/user/settings/emails/UserEmailSettings.tsx +++ /dev/null @@ -1,139 +0,0 @@ -/* eslint-disable react/jsx-no-bind */ -import React, { FunctionComponent, useEffect, useState, useCallback } from 'react' -import { RouteComponentProps } from 'react-router' -import * as H from 'history' - -import { queryGraphQL } from '../../../backend/graphql' -import { UserAreaUserFields } from '../../../graphql-operations' -import { gql } from '../../../../../shared/src/graphql/graphql' -import * as GQL from '../../../../../shared/src/graphql/schema' -import { createAggregateError } from '../../../../../shared/src/util/errors' - -import { eventLogger } from '../../../tracking/eventLogger' -import { ErrorAlert } from '../../../components/alerts' -import { PageTitle } from '../../../components/PageTitle' -import { UserEmail } from './UserEmail' -import { AddUserEmailForm } from './AddUserEmailForm' -import { SetUserPrimaryEmailForm } from './SetUserPrimaryEmailForm' - -interface Props extends RouteComponentProps<{}> { - user: UserAreaUserFields - history: H.History -} - -interface LoadingState { - loading: boolean - errorDescription: Error | null -} - -export const UserEmailSettings: FunctionComponent = ({ user, history }) => { - const [emails, setEmails] = useState([]) - const [status, setStatus] = useState({ loading: false, errorDescription: null }) - - const updateNewPrimaryEmail = (updatedEmail: string): GQL.IUserEmail[] => - emails.map(email => { - if (email.isPrimary && email.email !== updatedEmail) { - email.isPrimary = false - } - - if (email.email === updatedEmail) { - email.isPrimary = true - } - - return email - }) - - const onEmailVerify = ({ email: verifiedEmail, verified }: { email: string; verified: boolean }): void => { - const updatedEmails = emails.map(email => { - if (email.email === verifiedEmail) { - email.verified = verified - } - - return email - }) - - setEmails(updatedEmails) - } - - const onEmailRemove = (deletedEmail: string): void => { - setEmails(emails.filter(({ email }) => email !== deletedEmail)) - } - - const onPrimaryEmailSet = (email: string): void => { - setEmails(updateNewPrimaryEmail(email)) - } - - const fetchEmails = useCallback(async (): Promise => { - const { data, errors } = await queryGraphQL( - gql` - query UserEmails($user: ID!) { - node(id: $user) { - ... on User { - emails { - email - isPrimary - verified - verificationPending - viewerCanManuallyVerify - } - } - } - } - `, - { user: user.id } - ).toPromise() - - if (!data || (errors && errors.length > 0)) { - setStatus({ loading: false, errorDescription: createAggregateError(errors) }) - } else { - setStatus({ errorDescription: null, loading: false }) - const userResult = data.node as GQL.IUser - setEmails(userResult.emails) - } - }, [user]) - - useEffect(() => { - eventLogger.logViewEvent('UserSettingsEmails') - }, []) - - useEffect(() => { - console.log('EmailSettings') - // eslint-disable-next-line @typescript-eslint/no-floating-promises - fetchEmails() - }, [fetchEmails]) - - return ( -
- -

Emails

- {status.errorDescription && ( - - )} -
- {/* TODO: Fix this class */} -
    - {emails.map(email => ( -
  • - -
    • - ))} -
-
- -
- -
- ) -} diff --git a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx index 45da9196902c..2ef991030af9 100644 --- a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx +++ b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx @@ -1,272 +1,77 @@ -/* eslint rxjs/no-ignored-subscription: warn */ -import DeleteIcon from 'mdi-react/DeleteIcon' -import * as React from 'react' +/* eslint-disable react/jsx-no-bind */ +import React, { FunctionComponent, useEffect, useState, useCallback } from 'react' import { RouteComponentProps } from 'react-router' -import { Observable, Subject, Subscription } from 'rxjs' -import { map } from 'rxjs/operators' +import { LoadingSpinner } from '@sourcegraph/react-loading-spinner' +import * as H from 'history' +import { Subscription } from 'rxjs' + +import { queryGraphQL } from '../../../backend/graphql' +import { UserAreaUserFields } from '../../../graphql-operations' import { gql } from '../../../../../shared/src/graphql/graphql' import * as GQL from '../../../../../shared/src/graphql/schema' -import { createAggregateError, asError } from '../../../../../shared/src/util/errors' -import { mutateGraphQL, queryGraphQL } from '../../../backend/graphql' -import { FilteredConnection } from '../../../components/FilteredConnection' -import { PageTitle } from '../../../components/PageTitle' +import { createAggregateError } from '../../../../../shared/src/util/errors' import { SiteFlags } from '../../../site' import { siteFlags } from '../../../site/backend' + import { eventLogger } from '../../../tracking/eventLogger' -import { setUserEmailVerified } from '../backend' -import { AddUserEmailForm } from './AddUserEmailForm' import { ErrorAlert } from '../../../components/alerts' -import * as H from 'history' -import { UserAreaUserFields } from '../../../graphql-operations' +import { PageTitle } from '../../../components/PageTitle' +import { UserEmail } from './UserEmail' +import { AddUserEmailForm } from './AddUserEmailForm' +import { SetUserPrimaryEmailForm } from './SetUserPrimaryEmailForm' -interface UserEmailNodeProps { - node: GQL.IUserEmail +interface Props extends RouteComponentProps<{}> { user: UserAreaUserFields - - onDidUpdate: () => void history: H.History } -interface UserEmailNodeState { +interface LoadingState { loading: boolean - errorDescription?: string + errorDescription: Error | null } -class UserEmailNode extends React.PureComponent { - public state: UserEmailNodeState = { - loading: false, - } - - public render(): JSX.Element | null { - let verifiedFragment: React.ReactFragment - if (this.props.node.verified) { - verifiedFragment = Verified - } else if (this.props.node.verificationPending) { - verifiedFragment = Verification pending - } else { - verifiedFragment = Not verified - } - - return ( -
  • -
    -
    - {this.props.node.email}  {verifiedFragment}  - {this.props.node.isPrimary && Primary} -
    -
    - {' '} - {this.props.node.verified && !this.props.node.isPrimary && ( - - )}{' '} - {this.props.node.viewerCanManuallyVerify && ( - - )} -
    -
    - {this.state.errorDescription && ( - - )} -
    • - ) - } +export const UserSettingsEmailsPage: FunctionComponent = ({ user, history }) => { + const [emails, setEmails] = useState([]) + const [status, setStatus] = useState({ loading: false, errorDescription: null }) + const [flags, setFlags] = useState() - private setAsPrimary = (): void => { - this.setState({ - errorDescription: undefined, - loading: true, - }) - mutateGraphQL( - gql` - mutation SetUserEmailPrimary($user: ID!, $email: String!) { - setUserEmailPrimary(user: $user, email: $email) { - alwaysNil - } - } - `, - { user: this.props.user.id, email: this.props.node.email } - ) - .pipe( - map(({ data, errors }) => { - if (!data || (errors && errors.length > 0)) { - throw createAggregateError(errors) - } - }) - ) - .subscribe( - () => { - this.setState({ loading: false }) - eventLogger.log('UserEmailAddressSetAsPrimary') - if (this.props.onDidUpdate) { - this.props.onDidUpdate() - } - }, - error => this.setState({ loading: false, errorDescription: asError(error).message }) - ) - } + const updateNewPrimaryEmail = (updatedEmail: string): GQL.IUserEmail[] => + emails.map(email => { + if (email.isPrimary && email.email !== updatedEmail) { + email.isPrimary = false + } - private remove = (): void => { - if (!window.confirm(`Remove the email address ${this.props.node.email}?`)) { - return - } + if (email.email === updatedEmail) { + email.isPrimary = true + } - this.setState({ - errorDescription: undefined, - loading: true, + return email }) - mutateGraphQL( - gql` - mutation RemoveUserEmail($user: ID!, $email: String!) { - removeUserEmail(user: $user, email: $email) { - alwaysNil - } - } - `, - { user: this.props.user.id, email: this.props.node.email } - ) - .pipe( - map(({ data, errors }) => { - if (!data || (errors && errors.length > 0)) { - throw createAggregateError(errors) - } - }) - ) - .subscribe( - () => { - this.setState({ loading: false }) - eventLogger.log('UserEmailAddressDeleted') - if (this.props.onDidUpdate) { - this.props.onDidUpdate() - } - }, - error => this.setState({ loading: false, errorDescription: asError(error).message }) - ) - } - - private setAsVerified = (): void => this.setVerified(true) - private setAsUnverified = (): void => this.setVerified(false) + const onEmailVerify = ({ email: verifiedEmail, verified }: { email: string; verified: boolean }): void => { + const updatedEmails = emails.map(email => { + if (email.email === verifiedEmail) { + email.verified = verified + } - private setVerified(verified: boolean): void { - this.setState({ - errorDescription: undefined, - loading: true, + return email }) - // TODO this may call setState() after the component was unmounted - // eslint-disable-next-line rxjs/no-ignored-subscription - setUserEmailVerified(this.props.user.id, this.props.node.email, verified).subscribe( - () => { - this.setState({ loading: false }) - if (verified) { - eventLogger.log('UserEmailAddressMarkedVerified') - } else { - eventLogger.log('UserEmailAddressMarkedUnverified') - } - if (this.props.onDidUpdate) { - this.props.onDidUpdate() - } - }, - error => this.setState({ loading: false, errorDescription: asError(error).message }) - ) + setEmails(updatedEmails) } -} - -interface Props extends RouteComponentProps<{}> { - user: UserAreaUserFields - history: H.History -} - -interface State { - siteFlags?: SiteFlags -} - -/** We fake a XyzConnection type because our GraphQL API doesn't have one (or need one) for user emails. */ -interface UserEmailConnection { - nodes: GQL.IUserEmail[] - totalCount: number -} - -export class UserSettingsEmailsPage extends React.Component { - public state: State = {} - - private userEmailUpdates = new Subject() - private subscriptions = new Subscription() - public componentDidMount(): void { - eventLogger.logViewEvent('UserSettingsEmails') - - this.subscriptions.add(siteFlags.subscribe(siteFlags => this.setState({ siteFlags }))) + const onEmailRemove = (deletedEmail: string): void => { + setEmails(emails.filter(({ email }) => email !== deletedEmail)) } - public componentWillUnmount(): void { - this.subscriptions.unsubscribe() + const onPrimaryEmailSet = (email: string): void => { + setEmails(updateNewPrimaryEmail(email)) } - public render(): JSX.Element | null { - const nodeProps: Omit = { - user: this.props.user, - onDidUpdate: this.onDidUpdateUserEmail, - history: this.props.history, - } - - return ( -
    - -

    Emails

    - {this.state.siteFlags && !this.state.siteFlags.sendsEmailVerificationEmails && ( -
    - Sourcegraph is not configured to send email verifications. Newly added email addresses must be - manually verified by a site admin. -
    - )} - > - className="list-group list-group-flush mt-3" - noun="email address" - pluralNoun="email addresses" - queryConnection={this.queryUserEmails} - nodeComponent={UserEmailNode} - nodeComponentProps={nodeProps} - updates={this.userEmailUpdates} - hideSearch={true} - noSummaryIfAllNodesVisible={true} - history={this.props.history} - location={this.props.location} - /> - -
    - ) - } + const fetchEmails = useCallback(async (): Promise => { + setStatus({ errorDescription: null, loading: true }) - private queryUserEmails = (): Observable => - queryGraphQL( + const { data, errors } = await queryGraphQL( gql` query UserEmails($user: ID!) { node(id: $user) { @@ -282,22 +87,83 @@ export class UserSettingsEmailsPage extends React.Component { } } `, - { user: this.props.user.id } - ).pipe( - map(({ data, errors }) => { - if (!data || !data.node) { - throw createAggregateError(errors) - } - const user = data.node as GQL.IUser - if (!user.emails) { - throw createAggregateError(errors) - } - return { - nodes: user.emails, - totalCount: user.emails.length, - } - }) - ) + { user: user.id } + ).toPromise() + + if (!data || (errors && errors.length > 0)) { + setStatus({ loading: false, errorDescription: createAggregateError(errors) }) + } else { + setStatus({ errorDescription: null, loading: false }) + const userResult = data.node as GQL.IUser + setEmails(userResult.emails) + } + }, [user, setStatus, setEmails]) + + useEffect(() => { + eventLogger.logViewEvent('UserSettingsEmails') + const subscriptions = new Subscription() + subscriptions.add(siteFlags.subscribe(setFlags)) - private onDidUpdateUserEmail = (): void => this.userEmailUpdates.next() + return () => { + subscriptions.unsubscribe() + } + }, []) + + useEffect(() => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + fetchEmails() + }, [fetchEmails]) + + return ( +
    + +

    Emails

    + + {flags && !flags.sendsEmailVerificationEmails && ( +
    + Sourcegraph is not configured to send email verifications. Newly added email addresses must be + manually verified by a site admin. +
    + )} + + {status.errorDescription && ( + + )} + + {status.loading ? ( + + + + ) : ( +
    +
      + {emails.map(email => ( +
    • + +
      • + ))} +
    +
    + )} + + {/* re-fetch emails when new emails are added to guarantee correct state */} + +
    + {!status.loading && ( + + )} +
    + ) } diff --git a/client/web/src/user/settings/routes.tsx b/client/web/src/user/settings/routes.tsx index 45b6ce525a3e..41bb012dec2c 100644 --- a/client/web/src/user/settings/routes.tsx +++ b/client/web/src/user/settings/routes.tsx @@ -55,11 +55,6 @@ export const userSettingsAreaRoutes: readonly UserSettingsAreaRoute[] = [ { path: '/emails', exact: true, - render: lazyComponent(() => import('./emails/UserEmailSettings'), 'UserEmailSettings'), - }, - { - path: '/emails-o', - exact: true, render: lazyComponent(() => import('./emails/UserSettingsEmailsPage'), 'UserSettingsEmailsPage'), }, { From 14c077219f9c760d8495d0826c0c36145946ccb9 Mon Sep 17 00:00:00 2001 From: Artem Ruts <1319181+artemruts@users.noreply.github.com> Date: Wed, 18 Nov 2020 01:08:52 -0500 Subject: [PATCH 05/22] Formatting and lint fixes --- .../user/settings/emails/AddUserEmailForm.tsx | 2 +- .../emails/SetUserPrimaryEmailForm.tsx | 54 ++++++++++--------- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/client/web/src/user/settings/emails/AddUserEmailForm.tsx b/client/web/src/user/settings/emails/AddUserEmailForm.tsx index ec02543263c6..95eb35ca34b1 100644 --- a/client/web/src/user/settings/emails/AddUserEmailForm.tsx +++ b/client/web/src/user/settings/emails/AddUserEmailForm.tsx @@ -16,7 +16,7 @@ interface Props { user: GQL.ID /** Called after successfully adding an email to the user. */ - onDidAdd: (email: string) => void + onDidAdd: () => void className?: string history: H.History diff --git a/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx index e084166f6aa9..82d09eb13eb9 100644 --- a/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx +++ b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx @@ -1,5 +1,4 @@ -/* eslint-disable react/jsx-no-bind */ -import React, { useState, FunctionComponent, useEffect } from 'react' +import React, { useState, FunctionComponent, useEffect, useCallback } from 'react' import * as H from 'history' import { IUserEmail } from '../../../../../shared/src/graphql/schema' @@ -40,7 +39,7 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails * * 1. all emails that can be set as primary (not primary and verified) * 2. current primary email to be used as UI "placeholder" for disabled - * select + * select */ for (const email of emails) { // collect possible primary emails @@ -79,32 +78,35 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails const onPrimaryEmailSelect: React.ChangeEventHandler = event => setPrimaryEmail(event.target.value) - const onSubmit: React.FormEventHandler = async event => { - event.preventDefault() - setStatus({ loading: true, errorDescription: null }) - - const { data, errors } = await mutateGraphQL( - gql` - mutation SetUserEmailPrimary($user: ID!, $email: String!) { - setUserEmailPrimary(user: $user, email: $email) { - alwaysNil + const onSubmit: React.FormEventHandler = useCallback( + async event => { + event.preventDefault() + setStatus({ loading: true, errorDescription: null }) + + const { data, errors } = await mutateGraphQL( + gql` + mutation SetUserEmailPrimary($user: ID!, $email: String!) { + setUserEmailPrimary(user: $user, email: $email) { + alwaysNil + } } + `, + { user, email: primaryEmail } + ).toPromise() + + if (!data || (errors && errors.length > 0)) { + setStatus({ loading: false, errorDescription: createAggregateError(errors) }) + } else { + eventLogger.log('UserEmailAddressSetAsPrimary') + setStatus({ loading: false, errorDescription: null }) + + if (onDidSet) { + onDidSet(primaryEmail) } - `, - { user, email: primaryEmail } - ).toPromise() - - if (!data || (errors && errors.length > 0)) { - setStatus({ loading: false, errorDescription: createAggregateError(errors) }) - } else { - eventLogger.log('UserEmailAddressSetAsPrimary') - setStatus({ loading: false, errorDescription: null }) - - if (onDidSet) { - onDidSet(primaryEmail) } - } - } + }, + [user, primaryEmail, onDidSet] + ) return (
    From 92849c4a2cd2da13c993cfec1217d59a3b7b45a2 Mon Sep 17 00:00:00 2001 From: Artem Ruts <1319181+artemruts@users.noreply.github.com> Date: Thu, 19 Nov 2020 00:56:50 -0500 Subject: [PATCH 06/22] Addressing comments - 1 --- client/web/src/SourcegraphWebApp.scss | 1 + .../user/settings/emails/AddUserEmailForm.tsx | 6 +- .../emails/SetUserPrimaryEmailForm.tsx | 64 +++--- .../src/user/settings/emails/UserEmail.tsx | 190 +++++++----------- .../emails/UserSettingsEmailsPage.scss | 13 ++ .../emails/UserSettingsEmailsPage.tsx | 177 +++++++++------- 6 files changed, 228 insertions(+), 223 deletions(-) create mode 100644 client/web/src/user/settings/emails/UserSettingsEmailsPage.scss diff --git a/client/web/src/SourcegraphWebApp.scss b/client/web/src/SourcegraphWebApp.scss index 76e9f4ca634d..fb5b68c88040 100644 --- a/client/web/src/SourcegraphWebApp.scss +++ b/client/web/src/SourcegraphWebApp.scss @@ -117,6 +117,7 @@ body, @import '../../branded/src/components/tooltip/Tooltip'; @import './components/CtaBanner.scss'; @import './user/settings/UserSettingsArea'; +@import './user/settings/emails/UserSettingsEmailsPage.scss'; @import './site-admin/SiteAdminAlert'; @import './site/DockerForMacAlert'; @import './user/UserAvatar'; diff --git a/client/web/src/user/settings/emails/AddUserEmailForm.tsx b/client/web/src/user/settings/emails/AddUserEmailForm.tsx index 95eb35ca34b1..b0da146c9be2 100644 --- a/client/web/src/user/settings/emails/AddUserEmailForm.tsx +++ b/client/web/src/user/settings/emails/AddUserEmailForm.tsx @@ -42,7 +42,7 @@ export class AddUserEmailForm extends React.PureComponent { merge( of>({ error: undefined }), this.addUserEmail(this.state.email).pipe( - tap(() => this.props.onDidAdd(this.state.email)), + tap(() => this.props.onDidAdd()), map(() => ({ error: null, email: '' })), catchError(error => [{ error, email: this.state.email }]) ) @@ -64,7 +64,7 @@ export class AddUserEmailForm extends React.PureComponent { const loading = this.state.error === undefined return (
    -

    Add email address

    +

    Add email address

    + {status.errorDescription && ( + )} ) diff --git a/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss b/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss new file mode 100644 index 000000000000..c332e1dcd526 --- /dev/null +++ b/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss @@ -0,0 +1,13 @@ +.user-settings-emails-page { + &__list { + list-style-type: none; + margin-bottom: 0; + padding: 0; + } + &__btn { + padding: 0; + } + &__btn:not(:first-of-type) { + padding-left: 0.5rem; + } +} diff --git a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx index 2ef991030af9..020d3d2ab0b4 100644 --- a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx +++ b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx @@ -1,16 +1,13 @@ -/* eslint-disable react/jsx-no-bind */ import React, { FunctionComponent, useEffect, useState, useCallback } from 'react' import { RouteComponentProps } from 'react-router' import { LoadingSpinner } from '@sourcegraph/react-loading-spinner' import * as H from 'history' -import { Subscription } from 'rxjs' -import { queryGraphQL } from '../../../backend/graphql' -import { UserAreaUserFields } from '../../../graphql-operations' -import { gql } from '../../../../../shared/src/graphql/graphql' +import { requestGraphQL } from '../../../backend/graphql' +import { UserAreaUserFields, UserEmailsResult, UserEmailsVariables } from '../../../graphql-operations' +import { gql, dataOrThrowErrors } from '../../../../../shared/src/graphql/graphql' +import { useObservable } from '../../../../../shared/src/util/useObservable' import * as GQL from '../../../../../shared/src/graphql/schema' -import { createAggregateError } from '../../../../../shared/src/util/errors' -import { SiteFlags } from '../../../site' import { siteFlags } from '../../../site/backend' import { eventLogger } from '../../../tracking/eventLogger' @@ -27,91 +24,118 @@ interface Props extends RouteComponentProps<{}> { interface LoadingState { loading: boolean - errorDescription: Error | null + errorDescription?: Error } -export const UserSettingsEmailsPage: FunctionComponent = ({ user, history }) => { - const [emails, setEmails] = useState([]) - const [status, setStatus] = useState({ loading: false, errorDescription: null }) - const [flags, setFlags] = useState() +type UserEmail = Omit - const updateNewPrimaryEmail = (updatedEmail: string): GQL.IUserEmail[] => - emails.map(email => { - if (email.isPrimary && email.email !== updatedEmail) { - email.isPrimary = false - } +export const UserSettingsEmailsPage: FunctionComponent = ({ user, history }) => { + const [emails, setEmails] = useState([]) + const [status, setStatus] = useState({ loading: false }) + // const [flags, setFlags] = useState() + + const onEmailVerify = useCallback( + ({ email: verifiedEmail, verified }: { email: string; verified: boolean }): void => { + const updatedEmails = emails.map(email => { + if (email.email === verifiedEmail) { + email.verified = verified + } - if (email.email === updatedEmail) { - email.isPrimary = true - } + return email + }) - return email - }) + setEmails(updatedEmails) + }, + [emails] + ) - const onEmailVerify = ({ email: verifiedEmail, verified }: { email: string; verified: boolean }): void => { - const updatedEmails = emails.map(email => { - if (email.email === verifiedEmail) { - email.verified = verified - } + const onEmailRemove = useCallback( + (deletedEmail: string): void => { + setEmails(emails.filter(({ email }) => email !== deletedEmail)) + }, + [emails] + ) - return email - }) + const onPrimaryEmailSet = useCallback( + (email: string): void => { + const updateNewPrimaryEmail = (updatedEmail: string): UserEmail[] => + emails.map(email => { + if (email.isPrimary && email.email !== updatedEmail) { + email.isPrimary = false + } - setEmails(updatedEmails) - } + if (email.email === updatedEmail) { + email.isPrimary = true + } - const onEmailRemove = (deletedEmail: string): void => { - setEmails(emails.filter(({ email }) => email !== deletedEmail)) - } + return email + }) - const onPrimaryEmailSet = (email: string): void => { - setEmails(updateNewPrimaryEmail(email)) - } + setEmails(updateNewPrimaryEmail(email)) + }, + [emails] + ) const fetchEmails = useCallback(async (): Promise => { - setStatus({ errorDescription: null, loading: true }) - - const { data, errors } = await queryGraphQL( - gql` - query UserEmails($user: ID!) { - node(id: $user) { - ... on User { - emails { - email - isPrimary - verified - verificationPending - viewerCanManuallyVerify + setStatus({ loading: true }) + let fetchedEmails + + try { + fetchedEmails = dataOrThrowErrors( + await requestGraphQL( + gql` + query UserEmails($user: ID!) { + node(id: $user) { + ... on User { + emails { + email + isPrimary + verified + verificationPending + viewerCanManuallyVerify + } + } } } - } - } - `, - { user: user.id } - ).toPromise() - - if (!data || (errors && errors.length > 0)) { - setStatus({ loading: false, errorDescription: createAggregateError(errors) }) - } else { - setStatus({ errorDescription: null, loading: false }) - const userResult = data.node as GQL.IUser - setEmails(userResult.emails) + `, + { user: user.id } + ).toPromise() + ) + } catch (error) { + setStatus({ loading: false, errorDescription: error as Error }) } + + // TODO: check this logic + if (fetchedEmails?.node?.emails) { + setStatus({ loading: false }) + setEmails(fetchedEmails.node.emails) + } + + // if (!data || (errors && errors.length > 0)) { + // setStatus({ loading: false, errorDescription: createAggregateError(errors) }) + // } else { + // setStatus({ errorDescription: null, loading: false }) + // const userResult = data.node as GQL.IUser + // setEmails(userResult.emails) + // } }, [user, setStatus, setEmails]) - useEffect(() => { - eventLogger.logViewEvent('UserSettingsEmails') - const subscriptions = new Subscription() - subscriptions.add(siteFlags.subscribe(setFlags)) + const flags = useObservable(siteFlags) - return () => { - subscriptions.unsubscribe() - } - }, []) + // useEffect(() => { + // eventLogger.logViewEvent('UserSettingsEmails') + // const subscriptions = new Subscription() + // subscriptions.add(siteFlags.subscribe(setFlags)) + + // return () => { + // subscriptions.unsubscribe() + // } + // }, []) useEffect(() => { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - fetchEmails() + fetchEmails().catch(error => { + setStatus({ loading: false, errorDescription: error }) + }) }, [fetchEmails]) return ( @@ -135,10 +159,10 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history ) : ( -
    -
      +
      +
        {emails.map(email => ( -
      • +
      • = ({ user, history {/* re-fetch emails when new emails are added to guarantee correct state */} -
        +
        {!status.loading && ( Date: Thu, 19 Nov 2020 21:50:09 -0500 Subject: [PATCH 07/22] Addressing comments - 2 --- client/shared/src/util/useInputValidation.ts | 2 +- .../user/settings/emails/AddUserEmailForm.tsx | 190 +++++++++--------- .../src/user/settings/emails/UserEmail.tsx | 14 +- .../emails/UserSettingsEmailsPage.scss | 4 + .../emails/UserSettingsEmailsPage.tsx | 32 +-- 5 files changed, 120 insertions(+), 122 deletions(-) diff --git a/client/shared/src/util/useInputValidation.ts b/client/shared/src/util/useInputValidation.ts index af767d43dbcc..4c9163de41ea 100644 --- a/client/shared/src/util/useInputValidation.ts +++ b/client/shared/src/util/useInputValidation.ts @@ -38,7 +38,7 @@ export type InputValidationState = { value: string } & ( /** * React hook to manage validation of a single form input field. - * `useInputValidation` helps with coodinating the constraint validation API + * `useInputValidation` helps with coordinating the constraint validation API * and custom synchronous and asynchronous validators. * * @param options Config object that declares sync + async validators diff --git a/client/web/src/user/settings/emails/AddUserEmailForm.tsx b/client/web/src/user/settings/emails/AddUserEmailForm.tsx index b0da146c9be2..0e14ab2d888d 100644 --- a/client/web/src/user/settings/emails/AddUserEmailForm.tsx +++ b/client/web/src/user/settings/emails/AddUserEmailForm.tsx @@ -1,15 +1,18 @@ -import * as React from 'react' -import { merge, Observable, of, Subject, Subscription } from 'rxjs' -import { catchError, map, switchMap, tap } from 'rxjs/operators' -import { gql } from '../../../../../shared/src/graphql/graphql' +import React, { FunctionComponent, useMemo, useState } from 'react' +import classNames from 'classnames' + +import { gql, dataOrThrowErrors } from '../../../../../shared/src/graphql/graphql' import * as GQL from '../../../../../shared/src/graphql/schema' -import { createAggregateError, ErrorLike } from '../../../../../shared/src/util/errors' -import { mutateGraphQL } from '../../../backend/graphql' -import { Form } from '../../../../../branded/src/components/Form' +import { requestGraphQL } from '../../../backend/graphql' +import { asError, isErrorLike } from '../../../../../shared/src/util/errors' + +import { useInputValidation, deriveInputClassName } from '../../../../../shared/src/util/useInputValidation' + import { eventLogger } from '../../../tracking/eventLogger' import { ErrorAlert } from '../../../components/alerts' import { LoaderButton } from '../../../components/LoaderButton' import * as H from 'history' +import { AddUserEmailResult, AddUserEmailVariables } from '../../../graphql-operations' interface Props { /** The GraphQL ID of the user with whom the new emails are associated. */ @@ -23,96 +26,103 @@ interface Props { } interface State { - email: string - error?: ErrorLike | null + loadingOrError?: boolean | Error } -export class AddUserEmailForm extends React.PureComponent { - public state: State = { email: '', error: null } +export const AddUserEmailForm: FunctionComponent = ({ user, className, onDidAdd, history }) => { + const [status, setStatus] = useState({}) + + const [emailState, nextEmailFieldChange, emailInputReference] = useInputValidation( + useMemo( + () => ({ + synchronousValidators: [], + asynchronousValidators: [], + }), + [] + ) + ) - private submits = new Subject>() - private subscriptions = new Subscription() + const onSubmit: React.FormEventHandler = async event => { + event.preventDefault() - public componentDidMount(): void { - this.subscriptions.add( - this.submits - .pipe( - tap(event => event.preventDefault()), - switchMap(() => - merge( - of>({ error: undefined }), - this.addUserEmail(this.state.email).pipe( - tap(() => this.props.onDidAdd()), - map(() => ({ error: null, email: '' })), - catchError(error => [{ error, email: this.state.email }]) - ) - ) - ) - ) - .subscribe( - stateUpdate => this.setState(stateUpdate), - error => console.error(error) + if (emailState.kind === 'VALID') { + setStatus({ loadingOrError: true }) + + try { + dataOrThrowErrors( + await requestGraphQL( + gql` + mutation AddUserEmail($user: ID!, $email: String!) { + addUserEmail(user: $user, email: $email) { + alwaysNil + } + } + `, + { user, email: emailState.value } + ).toPromise() ) - ) - } + } catch (error) { + setStatus({ loadingOrError: asError(error) }) + return + } - public componentWillUnmount(): void { - this.subscriptions.unsubscribe() - } + eventLogger.log('NewUserEmailAddressAdded') + setStatus({}) - public render(): JSX.Element | null { - const loading = this.state.error === undefined - return ( -
        -

        Add email address

        -
        - - {' '} - - - {this.state.error && ( - - )} -
        - ) + if (onDidAdd) { + onDidAdd() + } + } } - private onChange: React.ChangeEventHandler = event => - this.setState({ email: event.currentTarget.value }) - private onSubmit: React.FormEventHandler = event => this.submits.next(event) - - private addUserEmail = (email: string): Observable => - mutateGraphQL( - gql` - mutation AddUserEmail($user: ID!, $email: String!) { - addUserEmail(user: $user, email: $email) { - alwaysNil - } - } - `, - { user: this.props.user, email } - ).pipe( - map(({ data, errors }) => { - if (!data || (errors && errors.length > 0)) { - throw createAggregateError(errors) - } - eventLogger.log('NewUserEmailAddressAdded') - }) - ) + return ( +
        + + {/* eslint-disable-next-line react/forbid-elements */} +
        + {' '} + + {emailState.kind === 'INVALID' && ( + + {emailState.reason} + + )} + + {isErrorLike(status.loadingOrError) && ( + + )} +
        + ) } diff --git a/client/web/src/user/settings/emails/UserEmail.tsx b/client/web/src/user/settings/emails/UserEmail.tsx index a052ec732652..9e07dcfbd2d8 100644 --- a/client/web/src/user/settings/emails/UserEmail.tsx +++ b/client/web/src/user/settings/emails/UserEmail.tsx @@ -4,6 +4,7 @@ import * as H from 'history' import { requestGraphQL } from '../../../backend/graphql' import { IUserEmail } from '../../../../../shared/src/graphql/schema' import { dataOrThrowErrors, gql } from '../../../../../shared/src/graphql/graphql' +import { asError } from '../../../../../shared/src/util/errors' import { eventLogger } from '../../../tracking/eventLogger' import { ErrorAlert } from '../../../components/alerts' @@ -31,7 +32,7 @@ interface Props { interface LoadingState { loading: boolean - errorDescription?: Error + error?: Error } export const UserEmail: FunctionComponent = ({ @@ -60,7 +61,7 @@ export const UserEmail: FunctionComponent = ({ ).toPromise() ) } catch (error) { - setStatus({ loading: false, errorDescription: error as Error }) + setStatus({ loading: false, error: asError(error) }) return } @@ -89,7 +90,7 @@ export const UserEmail: FunctionComponent = ({ ).toPromise() ) } catch (error) { - setStatus({ loading: false, errorDescription: error as Error }) + setStatus({ loading: false, error: asError(error) }) return } @@ -119,7 +120,8 @@ export const UserEmail: FunctionComponent = ({ <>
        - {email} {verifiedLinkFragment} {isPrimary && Primary} + {email} {verifiedLinkFragment}{' '} + {isPrimary && Primary}
        {viewerCanManuallyVerify && ( @@ -142,9 +144,7 @@ export const UserEmail: FunctionComponent = ({ )}
        - {status.errorDescription && ( - - )} + {status.error && } ) } diff --git a/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss b/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss index c332e1dcd526..46c109655336 100644 --- a/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss +++ b/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss @@ -10,4 +10,8 @@ &__btn:not(:first-of-type) { padding-left: 0.5rem; } + &__email { + display: inline-block; + margin-right: 0.25rem; + } } diff --git a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx index 020d3d2ab0b4..7522db6de857 100644 --- a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx +++ b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx @@ -9,6 +9,7 @@ import { gql, dataOrThrowErrors } from '../../../../../shared/src/graphql/graphq import { useObservable } from '../../../../../shared/src/util/useObservable' import * as GQL from '../../../../../shared/src/graphql/schema' import { siteFlags } from '../../../site/backend' +import { asError } from '../../../../../shared/src/util/errors' import { eventLogger } from '../../../tracking/eventLogger' import { ErrorAlert } from '../../../components/alerts' @@ -24,7 +25,7 @@ interface Props extends RouteComponentProps<{}> { interface LoadingState { loading: boolean - errorDescription?: Error + error?: Error } type UserEmail = Omit @@ -32,7 +33,6 @@ type UserEmail = Omit export const UserSettingsEmailsPage: FunctionComponent = ({ user, history }) => { const [emails, setEmails] = useState([]) const [status, setStatus] = useState({ loading: false }) - // const [flags, setFlags] = useState() const onEmailVerify = useCallback( ({ email: verifiedEmail, verified }: { email: string; verified: boolean }): void => { @@ -102,7 +102,7 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history ).toPromise() ) } catch (error) { - setStatus({ loading: false, errorDescription: error as Error }) + setStatus({ loading: false, error: asError(error) }) } // TODO: check this logic @@ -110,31 +110,17 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history setStatus({ loading: false }) setEmails(fetchedEmails.node.emails) } - - // if (!data || (errors && errors.length > 0)) { - // setStatus({ loading: false, errorDescription: createAggregateError(errors) }) - // } else { - // setStatus({ errorDescription: null, loading: false }) - // const userResult = data.node as GQL.IUser - // setEmails(userResult.emails) - // } }, [user, setStatus, setEmails]) const flags = useObservable(siteFlags) - // useEffect(() => { - // eventLogger.logViewEvent('UserSettingsEmails') - // const subscriptions = new Subscription() - // subscriptions.add(siteFlags.subscribe(setFlags)) - - // return () => { - // subscriptions.unsubscribe() - // } - // }, []) + useEffect(() => { + eventLogger.logViewEvent('UserSettingsEmails') + }, []) useEffect(() => { fetchEmails().catch(error => { - setStatus({ loading: false, errorDescription: error }) + setStatus({ loading: false, error: asError(error) }) }) }, [fetchEmails]) @@ -150,9 +136,7 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history
      )} - {status.errorDescription && ( - - )} + {status.error && } {status.loading ? ( From 01676bc1e18439944a0b8ed7d58e58dd67703fb8 Mon Sep 17 00:00:00 2001 From: Artem Ruts <1319181+artemruts@users.noreply.github.com> Date: Fri, 20 Nov 2020 01:09:34 -0500 Subject: [PATCH 08/22] Addressing comments - 3 --- .../user/settings/emails/AddUserEmailForm.tsx | 52 ++++++------- .../emails/SetUserPrimaryEmailForm.tsx | 76 +++++-------------- .../src/user/settings/emails/UserEmail.tsx | 2 + .../emails/UserSettingsEmailsPage.scss | 4 + .../emails/UserSettingsEmailsPage.tsx | 2 +- 5 files changed, 53 insertions(+), 83 deletions(-) diff --git a/client/web/src/user/settings/emails/AddUserEmailForm.tsx b/client/web/src/user/settings/emails/AddUserEmailForm.tsx index 0e14ab2d888d..8d1fa2e546f6 100644 --- a/client/web/src/user/settings/emails/AddUserEmailForm.tsx +++ b/client/web/src/user/settings/emails/AddUserEmailForm.tsx @@ -1,24 +1,21 @@ import React, { FunctionComponent, useMemo, useState } from 'react' import classNames from 'classnames' +import * as H from 'history' +import { AddUserEmailResult, AddUserEmailVariables } from '../../../graphql-operations' import { gql, dataOrThrowErrors } from '../../../../../shared/src/graphql/graphql' import * as GQL from '../../../../../shared/src/graphql/schema' import { requestGraphQL } from '../../../backend/graphql' import { asError, isErrorLike } from '../../../../../shared/src/util/errors' - import { useInputValidation, deriveInputClassName } from '../../../../../shared/src/util/useInputValidation' import { eventLogger } from '../../../tracking/eventLogger' import { ErrorAlert } from '../../../components/alerts' import { LoaderButton } from '../../../components/LoaderButton' -import * as H from 'history' -import { AddUserEmailResult, AddUserEmailVariables } from '../../../graphql-operations' +import { LoaderInput } from '../../../../../branded/src/components/LoaderInput' interface Props { - /** The GraphQL ID of the user with whom the new emails are associated. */ user: GQL.ID - - /** Called after successfully adding an email to the user. */ onDidAdd: () => void className?: string @@ -68,7 +65,6 @@ export const AddUserEmailForm: FunctionComponent = ({ user, className, on eventLogger.log('NewUserEmailAddressAdded') setStatus({}) - if (onDidAdd) { onDidAdd() } @@ -87,26 +83,28 @@ export const AddUserEmailForm: FunctionComponent = ({ user, className, on {/* eslint-disable-next-line react/forbid-elements */}
      - {' '} + + + {' '} = ({ user, emails, onDidSet, className, history }) => { - const [primaryEmail, setPrimaryEmail] = useState('') - const [emailOptions, setEmailOptions] = useState([]) + const [primaryEmail, setPrimaryEmail] = useState() const [status, setStatus] = useState({ loading: false }) - const [disabled, setDisabled] = useState(false) - useEffect(() => { - const possibleEmails = [] - let placeholderPrimaryEmail = '' - - /** - * Every time emails props changes, find: - * - * 1. all emails that can be set as primary (not primary and verified) - * 2. current primary email to be used as UI "placeholder" for disabled - * select - */ - for (const email of emails) { - // collect possible primary emails - if (!email.isPrimary && email.verified) { - possibleEmails.push(email.email) - } - - if (email.isPrimary) { - // there can be only one - placeholderPrimaryEmail = email.email - } - } - - const shouldDisable = possibleEmails.length === 0 - - let options - let selectValue - - /** - * If possible primary emails were found, use them as select's options - * and set the first email as select's value - */ - if (possibleEmails.length !== 0) { - options = possibleEmails - selectValue = possibleEmails[0] - } else { - options = [placeholderPrimaryEmail] - selectValue = placeholderPrimaryEmail + const options = emails.reduce((accumulator: string[], email) => { + if (!email.isPrimary && email.verified) { + accumulator.push(email.email) } + return accumulator + }, []) - setDisabled(shouldDisable) - setEmailOptions(options) - setPrimaryEmail(selectValue) - }, [emails]) + const defaultOption = emails.find(email => email.isPrimary)?.email + const disabled = options.length === 0 const onPrimaryEmailSelect: React.ChangeEventHandler = event => setPrimaryEmail(event.target.value) @@ -125,12 +88,17 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails onChange={onPrimaryEmailSelect} required={true} disabled={disabled} + defaultValue={defaultOption} > - {emailOptions.map(email => ( - - ))} + {disabled ? ( + + ) : ( + options.map(email => ( + + )) + )} {' '} = ({ user, emails className="btn btn-primary" /> - {status.errorDescription && ( - - )} + {status.error && }
    ) } diff --git a/client/web/src/user/settings/emails/UserEmail.tsx b/client/web/src/user/settings/emails/UserEmail.tsx index 9e07dcfbd2d8..f07a9f5bc948 100644 --- a/client/web/src/user/settings/emails/UserEmail.tsx +++ b/client/web/src/user/settings/emails/UserEmail.tsx @@ -129,6 +129,7 @@ export const UserEmail: FunctionComponent = ({ type="button" className="btn btn-link text-primary user-settings-emails-page__btn" onClick={() => updateEmailVerification(!verified)} + disabled={status.loading} > {verified ? 'Mark as unverified' : 'Mark as verified'} @@ -138,6 +139,7 @@ export const UserEmail: FunctionComponent = ({ type="button" className="btn btn-link text-danger user-settings-emails-page__btn" onClick={removeEmail} + disabled={status.loading} > Remove diff --git a/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss b/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss index 46c109655336..5c4b8ffe3ef4 100644 --- a/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss +++ b/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss @@ -14,4 +14,8 @@ display: inline-block; margin-right: 0.25rem; } + &__loader { + display: flex; + justify-content: center; + } } diff --git a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx index 7522db6de857..8e4a73bbf420 100644 --- a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx +++ b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx @@ -139,7 +139,7 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history {status.error && } {status.loading ? ( - + ) : ( From dc785a2fe985ae73638d82d0580d512d951781ef Mon Sep 17 00:00:00 2001 From: Artem Ruts <1319181+artemruts@users.noreply.github.com> Date: Sat, 21 Nov 2020 01:08:14 -0500 Subject: [PATCH 09/22] Minor tweaks to the select --- .../emails/SetUserPrimaryEmailForm.tsx | 45 +++++++++++-------- .../emails/UserSettingsEmailsPage.scss | 9 ++++ .../emails/UserSettingsEmailsPage.tsx | 12 +++-- 3 files changed, 45 insertions(+), 21 deletions(-) diff --git a/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx index e5a407203f11..c7522fd1a05b 100644 --- a/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx +++ b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx @@ -1,15 +1,16 @@ -import React, { useState, FunctionComponent, useCallback } from 'react' +import React, { useState, FunctionComponent, useCallback, useEffect } from 'react' import * as H from 'history' import { IUserEmail } from '../../../../../shared/src/graphql/schema' import { requestGraphQL } from '../../../backend/graphql' import { gql, dataOrThrowErrors } from '../../../../../shared/src/graphql/graphql' +import { SetUserEmailPrimaryResult, SetUserEmailPrimaryVariables } from '../../../graphql-operations' import { eventLogger } from '../../../tracking/eventLogger' +import { asError } from '../../../../../shared/src/util/errors' import { Form } from '../../../../../branded/src/components/Form' import { LoaderButton } from '../../../components/LoaderButton' import { ErrorAlert } from '../../../components/alerts' -import { SetUserEmailPrimaryResult, SetUserEmailPrimaryVariables } from '../../../graphql-operations' interface Props { user: string @@ -25,18 +26,27 @@ interface UserEmailState { } export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails, onDidSet, className, history }) => { - const [primaryEmail, setPrimaryEmail] = useState() + const [primaryEmail, setPrimaryEmail] = useState('') + const [options, setOptions] = useState([]) const [status, setStatus] = useState({ loading: false }) - const options = emails.reduce((accumulator: string[], email) => { - if (!email.isPrimary && email.verified) { - accumulator.push(email.email) - } - return accumulator - }, []) + useEffect(() => { + const options = emails.reduce((accumulator: string[], email) => { + if (!email.isPrimary && email.verified) { + accumulator.push(email.email) + } + return accumulator + }, []) + + setOptions(options) - const defaultOption = emails.find(email => email.isPrimary)?.email - const disabled = options.length === 0 + const defaultOption = emails.find(email => email.isPrimary)?.email + /** + * If there are options, non-primary and verified emails + * Use the first value as default value on the initial render + */ + setPrimaryEmail(options[0] || defaultOption || '') + }, [emails]) const onPrimaryEmailSelect: React.ChangeEventHandler = event => setPrimaryEmail(event.target.value) @@ -60,7 +70,7 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails ).toPromise() ) } catch (error) { - setStatus({ loading: false, errorDescription: error as Error }) + setStatus({ loading: false, error: asError(error) }) return } @@ -87,11 +97,10 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails value={primaryEmail} onChange={onPrimaryEmailSelect} required={true} - disabled={disabled} - defaultValue={defaultOption} + disabled={options.length === 0} > - {disabled ? ( - + {options.length === 0 ? ( + ) : ( options.map(email => ( )) )} - {' '} + diff --git a/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss b/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss index 5c4b8ffe3ef4..d4ebb9b2ac90 100644 --- a/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss +++ b/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss @@ -1,4 +1,13 @@ .user-settings-emails-page { + .form-inline input:not(.is-invalid), + .form-inline input:not(.is-valid) { + padding-right: calc(1.5em + 0.75rem); + } + .loader-input { + &__spinner { + right: 1rem; + } + } &__list { list-style-type: none; margin-bottom: 0; diff --git a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx index 8e4a73bbf420..d29769f2bd19 100644 --- a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx +++ b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx @@ -105,7 +105,6 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history setStatus({ loading: false, error: asError(error) }) } - // TODO: check this logic if (fetchedEmails?.node?.emails) { setStatus({ loading: false }) setEmails(fetchedEmails.node.emails) @@ -160,8 +159,15 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history
    )} - {/* re-fetch emails when new emails are added to guarantee correct state */} - + {/* re-fetch emails on onDidAdd to guarantee correct state */} + {/* Note: key - is a workaround to clear state in useInputValidation hook when the email is added */} +
    {!status.loading && ( Date: Sat, 21 Nov 2020 01:56:58 -0500 Subject: [PATCH 10/22] Adjust padding --- client/web/src/user/settings/emails/UserSettingsEmailsPage.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss b/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss index d4ebb9b2ac90..0d43bd4b774c 100644 --- a/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss +++ b/client/web/src/user/settings/emails/UserSettingsEmailsPage.scss @@ -1,7 +1,7 @@ .user-settings-emails-page { .form-inline input:not(.is-invalid), .form-inline input:not(.is-valid) { - padding-right: calc(1.5em + 0.75rem); + padding-right: 2rem; } .loader-input { &__spinner { From 09863eb409ad1f8fcf503e21ecf1e81f702fbc25 Mon Sep 17 00:00:00 2001 From: Artem Ruts <1319181+artemruts@users.noreply.github.com> Date: Mon, 23 Nov 2020 12:57:26 -0500 Subject: [PATCH 11/22] Simplifying code --- .../user/settings/emails/AddUserEmailForm.tsx | 39 ++++---- .../emails/SetUserPrimaryEmailForm.tsx | 90 ++++++++----------- .../src/user/settings/emails/UserEmail.tsx | 73 +++++++-------- .../emails/UserSettingsEmailsPage.tsx | 88 +++++------------- 4 files changed, 106 insertions(+), 184 deletions(-) diff --git a/client/web/src/user/settings/emails/AddUserEmailForm.tsx b/client/web/src/user/settings/emails/AddUserEmailForm.tsx index 8d1fa2e546f6..30a45e764a15 100644 --- a/client/web/src/user/settings/emails/AddUserEmailForm.tsx +++ b/client/web/src/user/settings/emails/AddUserEmailForm.tsx @@ -4,9 +4,8 @@ import * as H from 'history' import { AddUserEmailResult, AddUserEmailVariables } from '../../../graphql-operations' import { gql, dataOrThrowErrors } from '../../../../../shared/src/graphql/graphql' -import * as GQL from '../../../../../shared/src/graphql/schema' import { requestGraphQL } from '../../../backend/graphql' -import { asError, isErrorLike } from '../../../../../shared/src/util/errors' +import { asError, isErrorLike, ErrorLike } from '../../../../../shared/src/util/errors' import { useInputValidation, deriveInputClassName } from '../../../../../shared/src/util/useInputValidation' import { eventLogger } from '../../../tracking/eventLogger' @@ -15,19 +14,17 @@ import { LoaderButton } from '../../../components/LoaderButton' import { LoaderInput } from '../../../../../branded/src/components/LoaderInput' interface Props { - user: GQL.ID + user: string onDidAdd: () => void + history: H.History className?: string - history: H.History } -interface State { - loadingOrError?: boolean | Error -} +type Status = undefined | 'loading' | ErrorLike export const AddUserEmailForm: FunctionComponent = ({ user, className, onDidAdd, history }) => { - const [status, setStatus] = useState({}) + const [statusOrError, setStatusOrError] = useState() const [emailState, nextEmailFieldChange, emailInputReference] = useInputValidation( useMemo( @@ -43,7 +40,7 @@ export const AddUserEmailForm: FunctionComponent = ({ user, className, on event.preventDefault() if (emailState.kind === 'VALID') { - setStatus({ loadingOrError: true }) + setStatusOrError('loading') try { dataOrThrowErrors( @@ -58,15 +55,15 @@ export const AddUserEmailForm: FunctionComponent = ({ user, className, on { user, email: emailState.value } ).toPromise() ) - } catch (error) { - setStatus({ loadingOrError: asError(error) }) - return - } - eventLogger.log('NewUserEmailAddressAdded') - setStatus({}) - if (onDidAdd) { - onDidAdd() + eventLogger.log('NewUserEmailAddressAdded') + setStatusOrError(undefined) + + if (onDidAdd) { + onDidAdd() + } + } catch (error) { + setStatusOrError(asError(error)) } } } @@ -106,10 +103,10 @@ export const AddUserEmailForm: FunctionComponent = ({ user, className, on /> {' '} {emailState.kind === 'INVALID' && ( @@ -118,9 +115,7 @@ export const AddUserEmailForm: FunctionComponent = ({ user, className, on )} - {isErrorLike(status.loadingOrError) && ( - - )} + {isErrorLike(statusOrError) && }
    ) } diff --git a/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx index c7522fd1a05b..ef94d1b616e8 100644 --- a/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx +++ b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx @@ -1,52 +1,43 @@ -import React, { useState, FunctionComponent, useCallback, useEffect } from 'react' +import React, { useState, FunctionComponent, useCallback } from 'react' import * as H from 'history' -import { IUserEmail } from '../../../../../shared/src/graphql/schema' import { requestGraphQL } from '../../../backend/graphql' import { gql, dataOrThrowErrors } from '../../../../../shared/src/graphql/graphql' -import { SetUserEmailPrimaryResult, SetUserEmailPrimaryVariables } from '../../../graphql-operations' +import { SetUserEmailPrimaryResult, SetUserEmailPrimaryVariables, UserEmailsResult } from '../../../graphql-operations' import { eventLogger } from '../../../tracking/eventLogger' -import { asError } from '../../../../../shared/src/util/errors' +import { asError, ErrorLike, isErrorLike } from '../../../../../shared/src/util/errors' import { Form } from '../../../../../branded/src/components/Form' import { LoaderButton } from '../../../components/LoaderButton' import { ErrorAlert } from '../../../components/alerts' +type UserEmail = NonNullable['emails'][number] + interface Props { user: string - emails: Omit[] - onDidSet: (email: string) => void + emails: UserEmail[] + onDidSet: () => void history: H.History - className?: string -} -interface UserEmailState { - loading: boolean - error?: Error + className?: string } -export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails, onDidSet, className, history }) => { - const [primaryEmail, setPrimaryEmail] = useState('') - const [options, setOptions] = useState([]) - const [status, setStatus] = useState({ loading: false }) +type Status = undefined | 'loading' | ErrorLike - useEffect(() => { - const options = emails.reduce((accumulator: string[], email) => { - if (!email.isPrimary && email.verified) { - accumulator.push(email.email) - } - return accumulator - }, []) +// There is always exactly one primary email returned from the backend +// eslint-disable-next-line @typescript-eslint/no-non-null-assertion +const findPrimaryEmail = (emails: UserEmail[]): string => emails.find(email => email.isPrimary)!.email - setOptions(options) +export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails, onDidSet, className, history }) => { + const [primaryEmail, setPrimaryEmail] = useState(findPrimaryEmail(emails)) + const [statusOrError, setStatusOrError] = useState() - const defaultOption = emails.find(email => email.isPrimary)?.email - /** - * If there are options, non-primary and verified emails - * Use the first value as default value on the initial render - */ - setPrimaryEmail(options[0] || defaultOption || '') - }, [emails]) + const options = emails.reduce((accumulator: string[], email) => { + if (email.verified) { + accumulator.push(email.email) + } + return accumulator + }, []) const onPrimaryEmailSelect: React.ChangeEventHandler = event => setPrimaryEmail(event.target.value) @@ -54,7 +45,7 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails const onSubmit: React.FormEventHandler = useCallback( async event => { event.preventDefault() - setStatus({ loading: true }) + setStatusOrError('loading') try { dataOrThrowErrors( @@ -69,16 +60,15 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails { user, email: primaryEmail } ).toPromise() ) - } catch (error) { - setStatus({ loading: false, error: asError(error) }) - return - } - eventLogger.log('UserEmailAddressSetAsPrimary') - setStatus({ loading: false }) + eventLogger.log('UserEmailAddressSetAsPrimary') + setStatusOrError(undefined) - if (onDidSet) { - onDidSet(primaryEmail) + if (onDidSet) { + onDidSet() + } + } catch (error) { + setStatusOrError(asError(error)) } }, [user, primaryEmail, onDidSet] @@ -97,27 +87,23 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails value={primaryEmail} onChange={onPrimaryEmailSelect} required={true} - disabled={options.length === 0} + disabled={options.length === 1 || statusOrError === 'loading'} > - {options.length === 0 ? ( - - ) : ( - options.map(email => ( - - )) - )} + {options.map(email => ( + + ))} - {status.error && } + {isErrorLike(statusOrError) && }
    ) } diff --git a/client/web/src/user/settings/emails/UserEmail.tsx b/client/web/src/user/settings/emails/UserEmail.tsx index f07a9f5bc948..cfb0223e2918 100644 --- a/client/web/src/user/settings/emails/UserEmail.tsx +++ b/client/web/src/user/settings/emails/UserEmail.tsx @@ -2,38 +2,29 @@ import React, { useState, useCallback, FunctionComponent } from 'react' import * as H from 'history' import { requestGraphQL } from '../../../backend/graphql' -import { IUserEmail } from '../../../../../shared/src/graphql/schema' import { dataOrThrowErrors, gql } from '../../../../../shared/src/graphql/graphql' -import { asError } from '../../../../../shared/src/util/errors' - -import { eventLogger } from '../../../tracking/eventLogger' -import { ErrorAlert } from '../../../components/alerts' - import { + UserEmailsResult, RemoveUserEmailResult, RemoveUserEmailVariables, SetUserEmailVerifiedResult, SetUserEmailVerifiedVariables, } from '../../../graphql-operations' +import { asError, ErrorLike, isErrorLike } from '../../../../../shared/src/util/errors' +import { eventLogger } from '../../../tracking/eventLogger' -interface VerificationUpdate { - email: string - verified: boolean -} +import { ErrorAlert } from '../../../components/alerts' interface Props { user: string - email: Omit + email: NonNullable['emails'][number] history: H.History onDidRemove?: (email: string) => void - onEmailVerify?: (update: VerificationUpdate) => void + onEmailVerify?: () => void } -interface LoadingState { - loading: boolean - error?: Error -} +type Status = undefined | 'loading' | ErrorLike export const UserEmail: FunctionComponent = ({ user, @@ -42,10 +33,10 @@ export const UserEmail: FunctionComponent = ({ onEmailVerify, history, }) => { - const [status, setStatus] = useState({ loading: false }) + const [statusOrError, setStatusOrError] = useState() const removeEmail = useCallback(async (): Promise => { - setStatus({ loading: true }) + setStatusOrError('loading') try { dataOrThrowErrors( @@ -60,21 +51,20 @@ export const UserEmail: FunctionComponent = ({ { user, email } ).toPromise() ) - } catch (error) { - setStatus({ loading: false, error: asError(error) }) - return - } - setStatus({ loading: false }) - eventLogger.log('UserEmailAddressDeleted') + setStatusOrError(undefined) + eventLogger.log('UserEmailAddressDeleted') - if (onDidRemove) { - onDidRemove(email) + if (onDidRemove) { + onDidRemove(email) + } + } catch (error) { + setStatusOrError(asError(error)) } }, [email, user, onDidRemove]) const updateEmailVerification = async (verified: boolean): Promise => { - setStatus({ ...status, loading: true }) + setStatusOrError('loading') try { dataOrThrowErrors( @@ -89,21 +79,20 @@ export const UserEmail: FunctionComponent = ({ { user, email, verified } ).toPromise() ) - } catch (error) { - setStatus({ loading: false, error: asError(error) }) - return - } - setStatus({ loading: false }) + setStatusOrError(undefined) - if (verified) { - eventLogger.log('UserEmailAddressMarkedVerified') - } else { - eventLogger.log('UserEmailAddressMarkedUnverified') - } + if (verified) { + eventLogger.log('UserEmailAddressMarkedVerified') + } else { + eventLogger.log('UserEmailAddressMarkedUnverified') + } - if (onEmailVerify) { - onEmailVerify({ email, verified }) + if (onEmailVerify) { + onEmailVerify() + } + } catch (error) { + setStatusOrError(asError(error)) } } @@ -129,7 +118,7 @@ export const UserEmail: FunctionComponent = ({ type="button" className="btn btn-link text-primary user-settings-emails-page__btn" onClick={() => updateEmailVerification(!verified)} - disabled={status.loading} + disabled={statusOrError === 'loading'} > {verified ? 'Mark as unverified' : 'Mark as verified'} @@ -139,14 +128,14 @@ export const UserEmail: FunctionComponent = ({ type="button" className="btn btn-link text-danger user-settings-emails-page__btn" onClick={removeEmail} - disabled={status.loading} + disabled={statusOrError === 'loading'} > Remove )} - {status.error && } + {isErrorLike(statusOrError) && } ) } diff --git a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx index d29769f2bd19..eddfafd4e9c9 100644 --- a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx +++ b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx @@ -7,11 +7,10 @@ import { requestGraphQL } from '../../../backend/graphql' import { UserAreaUserFields, UserEmailsResult, UserEmailsVariables } from '../../../graphql-operations' import { gql, dataOrThrowErrors } from '../../../../../shared/src/graphql/graphql' import { useObservable } from '../../../../../shared/src/util/useObservable' -import * as GQL from '../../../../../shared/src/graphql/schema' import { siteFlags } from '../../../site/backend' -import { asError } from '../../../../../shared/src/util/errors' - +import { asError, ErrorLike, isErrorLike } from '../../../../../shared/src/util/errors' import { eventLogger } from '../../../tracking/eventLogger' + import { ErrorAlert } from '../../../components/alerts' import { PageTitle } from '../../../components/PageTitle' import { UserEmail } from './UserEmail' @@ -23,31 +22,12 @@ interface Props extends RouteComponentProps<{}> { history: H.History } -interface LoadingState { - loading: boolean - error?: Error -} - -type UserEmail = Omit +type UserEmail = NonNullable['emails'][number] +type Status = undefined | 'loading' | 'loaded' | ErrorLike export const UserSettingsEmailsPage: FunctionComponent = ({ user, history }) => { const [emails, setEmails] = useState([]) - const [status, setStatus] = useState({ loading: false }) - - const onEmailVerify = useCallback( - ({ email: verifiedEmail, verified }: { email: string; verified: boolean }): void => { - const updatedEmails = emails.map(email => { - if (email.email === verifiedEmail) { - email.verified = verified - } - - return email - }) - - setEmails(updatedEmails) - }, - [emails] - ) + const [statusOrError, setStatusOrError] = useState() const onEmailRemove = useCallback( (deletedEmail: string): void => { @@ -56,32 +36,11 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history [emails] ) - const onPrimaryEmailSet = useCallback( - (email: string): void => { - const updateNewPrimaryEmail = (updatedEmail: string): UserEmail[] => - emails.map(email => { - if (email.isPrimary && email.email !== updatedEmail) { - email.isPrimary = false - } - - if (email.email === updatedEmail) { - email.isPrimary = true - } - - return email - }) - - setEmails(updateNewPrimaryEmail(email)) - }, - [emails] - ) - const fetchEmails = useCallback(async (): Promise => { - setStatus({ loading: true }) - let fetchedEmails + setStatusOrError('loading') try { - fetchedEmails = dataOrThrowErrors( + const fetchedEmails = dataOrThrowErrors( await requestGraphQL( gql` query UserEmails($user: ID!) { @@ -101,15 +60,15 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history { user: user.id } ).toPromise() ) - } catch (error) { - setStatus({ loading: false, error: asError(error) }) - } - if (fetchedEmails?.node?.emails) { - setStatus({ loading: false }) - setEmails(fetchedEmails.node.emails) + if (fetchedEmails?.node?.emails) { + setEmails(fetchedEmails.node.emails) + setStatusOrError('loaded') + } + } catch (error) { + setStatusOrError(asError(error)) } - }, [user, setStatus, setEmails]) + }, [user, setStatusOrError, setEmails]) const flags = useObservable(siteFlags) @@ -119,7 +78,7 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history useEffect(() => { fetchEmails().catch(error => { - setStatus({ loading: false, error: asError(error) }) + setStatusOrError(asError(error)) }) }, [fetchEmails]) @@ -135,9 +94,9 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history )} - {status.error && } + {isErrorLike(statusOrError) && } - {status.loading ? ( + {statusOrError === 'loading' ? ( @@ -149,7 +108,7 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history @@ -159,8 +118,6 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history )} - {/* re-fetch emails on onDidAdd to guarantee correct state */} - {/* Note: key - is a workaround to clear state in useInputValidation hook when the email is added */} = ({ user, history history={history} />
    - {!status.loading && ( - + {statusOrError === 'loaded' && ( + )} ) From 4330f6aa1f99733a663cee6b2df74b94e61dbbb9 Mon Sep 17 00:00:00 2001 From: TJ Kandala Date: Mon, 23 Nov 2020 15:51:14 -0500 Subject: [PATCH 12/22] Allow consumers to override useInputValidation state, use in AddUserEmailForm (#16065) --- client/shared/src/util/useInputValidation.ts | 46 ++++++++++++++++--- .../user/settings/emails/AddUserEmailForm.tsx | 3 +- .../emails/UserSettingsEmailsPage.tsx | 9 +--- 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/client/shared/src/util/useInputValidation.ts b/client/shared/src/util/useInputValidation.ts index 4c9163de41ea..bc3577925abb 100644 --- a/client/shared/src/util/useInputValidation.ts +++ b/client/shared/src/util/useInputValidation.ts @@ -1,5 +1,5 @@ -import { compact, head } from 'lodash' -import { useMemo, useState, useRef } from 'react' +import { compact, head, noop } from 'lodash' +import { useMemo, useState, useRef, useCallback } from 'react' import { concat, EMPTY, Observable, of, zip } from 'rxjs' import { catchError, map, switchMap, tap, debounceTime } from 'rxjs/operators' import { useEventObservable } from './useObservable' @@ -36,22 +36,32 @@ export type InputValidationState = { value: string } & ( | { kind: 'INVALID'; reason: string } ) +/** + * Options for overriding input state programmatically. If `overrideState` is called without + * these options, the input will be cleared and not validated. + */ +interface OverrideOptions { + /** The value to set input state to */ + value: string + + /** Whether to validate the new value */ + validate?: boolean +} + /** * React hook to manage validation of a single form input field. * `useInputValidation` helps with coordinating the constraint validation API * and custom synchronous and asynchronous validators. * * @param options Config object that declares sync + async validators - * @param initialValue - * - * @returns */ export function useInputValidation( options: ValidationOptions ): [ InputValidationState, (change: React.ChangeEvent) => void, - React.MutableRefObject + React.MutableRefObject, + (overrideOptions: OverrideOptions) => void ] { const inputReference = useRef(null) @@ -66,7 +76,29 @@ export function useInputValidation( const [nextInputChangeEvent] = useEventObservable(validationPipeline) - return [inputState, nextInputChangeEvent, inputReference] + // TODO(tj): Move control of state to consumer + const overrideState = useCallback( + (overrideOptions: OverrideOptions) => { + // clear custom validity + inputReference.current?.setCustomValidity('') + + // clear React state + setInputState({ + kind: overrideOptions?.validate ? 'LOADING' : 'NOT_VALIDATED', + value: overrideOptions?.value ?? '', + }) + + if (overrideOptions?.validate) { + nextInputChangeEvent({ + preventDefault: noop, + target: { value: overrideOptions.value }, + }) + } + }, + [nextInputChangeEvent] + ) + + return [inputState, nextInputChangeEvent, inputReference, overrideState] } /** diff --git a/client/web/src/user/settings/emails/AddUserEmailForm.tsx b/client/web/src/user/settings/emails/AddUserEmailForm.tsx index 30a45e764a15..4235e024b6c2 100644 --- a/client/web/src/user/settings/emails/AddUserEmailForm.tsx +++ b/client/web/src/user/settings/emails/AddUserEmailForm.tsx @@ -26,7 +26,7 @@ type Status = undefined | 'loading' | ErrorLike export const AddUserEmailForm: FunctionComponent = ({ user, className, onDidAdd, history }) => { const [statusOrError, setStatusOrError] = useState() - const [emailState, nextEmailFieldChange, emailInputReference] = useInputValidation( + const [emailState, nextEmailFieldChange, emailInputReference, overrideEmailState] = useInputValidation( useMemo( () => ({ synchronousValidators: [], @@ -57,6 +57,7 @@ export const AddUserEmailForm: FunctionComponent = ({ user, className, on ) eventLogger.log('NewUserEmailAddressAdded') + overrideEmailState({ value: '' }) setStatusOrError(undefined) if (onDidAdd) { diff --git a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx index eddfafd4e9c9..93d274577845 100644 --- a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx +++ b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx @@ -118,13 +118,8 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history )} - + {/* re-fetch emails on onDidAdd to guarantee correct state */} +
    {statusOrError === 'loaded' && ( From 391ca02f033c48c836f4ba3f317c436608f6631d Mon Sep 17 00:00:00 2001 From: Artem Ruts <1319181+artemruts@users.noreply.github.com> Date: Wed, 25 Nov 2020 12:05:11 -0500 Subject: [PATCH 13/22] Remove hook changes and simplify css --- .../src/user/settings/emails/AddUserEmailForm.tsx | 1 - client/web/src/user/settings/emails/UserEmail.tsx | 8 ++++---- .../settings/emails/UserSettingsEmailsPage.scss | 14 +------------- .../settings/emails/UserSettingsEmailsPage.tsx | 8 ++++---- 4 files changed, 9 insertions(+), 22 deletions(-) diff --git a/client/web/src/user/settings/emails/AddUserEmailForm.tsx b/client/web/src/user/settings/emails/AddUserEmailForm.tsx index 4235e024b6c2..3b6138166a99 100644 --- a/client/web/src/user/settings/emails/AddUserEmailForm.tsx +++ b/client/web/src/user/settings/emails/AddUserEmailForm.tsx @@ -95,7 +95,6 @@ export const AddUserEmailForm: FunctionComponent = ({ user, className, on value={emailState.value} ref={emailInputReference} required={true} - placeholder="Email" autoComplete="email" autoCorrect="off" autoCapitalize="off" diff --git a/client/web/src/user/settings/emails/UserEmail.tsx b/client/web/src/user/settings/emails/UserEmail.tsx index cfb0223e2918..ed6937582837 100644 --- a/client/web/src/user/settings/emails/UserEmail.tsx +++ b/client/web/src/user/settings/emails/UserEmail.tsx @@ -109,14 +109,14 @@ export const UserEmail: FunctionComponent = ({ <>
    - {email} {verifiedLinkFragment}{' '} - {isPrimary && Primary} + {email} + {verifiedLinkFragment} {isPrimary && Primary}
    {viewerCanManuallyVerify && ( - )} + )}{' '} {!isPrimary && ( -

    - - - -

    - - ) - }} + {() => ( +
    + + + +
    + )} )) diff --git a/client/branded/src/global-styles/index.scss b/client/branded/src/global-styles/index.scss index 26d941e974cf..cc225b6075e8 100644 --- a/client/branded/src/global-styles/index.scss +++ b/client/branded/src/global-styles/index.scss @@ -136,6 +136,7 @@ $hr-margin-y: 0.25rem; @import './forms'; @import './highlight'; @import './web-content'; +@import '~@sourcegraph/react-loading-spinner/lib/LoadingSpinner.css'; * { box-sizing: border-box; diff --git a/client/browser/src/branded.scss b/client/browser/src/branded.scss index 87528c4e3dfa..4d217f9b8090 100644 --- a/client/browser/src/branded.scss +++ b/client/browser/src/branded.scss @@ -6,4 +6,3 @@ @import './browser-extension/after-install-page/AfterInstallPageContent'; @import './browser-extension/options-menu/OptionsPage'; @import '../../branded/src/components/LoaderInput.scss'; -@import '~@sourcegraph/react-loading-spinner/lib/LoadingSpinner.css'; diff --git a/client/web/src/SourcegraphWebApp.scss b/client/web/src/SourcegraphWebApp.scss index fb5b68c88040..c3f52f709779 100644 --- a/client/web/src/SourcegraphWebApp.scss +++ b/client/web/src/SourcegraphWebApp.scss @@ -137,7 +137,6 @@ body, @import '../../branded/src/components/panel/views/FileLocations'; @import '../../branded/src/components/panel/views/HierarchicalLocationsView'; @import '../../branded/src/components/LoaderInput'; -@import '~@sourcegraph/react-loading-spinner/lib/LoadingSpinner.css'; @import '../../shared/src/index'; diff --git a/client/web/src/user/settings/emails/AddUserEmailForm.tsx b/client/web/src/user/settings/emails/AddUserEmailForm.tsx index d94df1986930..676084970fda 100644 --- a/client/web/src/user/settings/emails/AddUserEmailForm.tsx +++ b/client/web/src/user/settings/emails/AddUserEmailForm.tsx @@ -81,13 +81,16 @@ export const AddUserEmailForm: FunctionComponent = ({ user, className, on {/* eslint-disable-next-line react/forbid-elements */}
    - + Date: Fri, 27 Nov 2020 16:34:01 -0500 Subject: [PATCH 18/22] dedup storybook import --- client/branded/src/components/LoaderInput.story.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/client/branded/src/components/LoaderInput.story.tsx b/client/branded/src/components/LoaderInput.story.tsx index bea167fc80b3..e04e91dfcce3 100644 --- a/client/branded/src/components/LoaderInput.story.tsx +++ b/client/branded/src/components/LoaderInput.story.tsx @@ -3,7 +3,6 @@ import { storiesOf } from '@storybook/react' import React from 'react' import { LoaderInput } from './LoaderInput' import { BrandedStory } from './BrandedStory' -import { boolean } from '@storybook/addon-knobs' const { add } = storiesOf('branded/LoaderInput', module).addDecorator(story => (
    From 633650b859fc6987e91073ac3215c09714bcfdb5 Mon Sep 17 00:00:00 2001 From: TJ Kandala Date: Mon, 30 Nov 2020 10:48:21 -0500 Subject: [PATCH 19/22] fix LoaderInput story --- client/branded/src/components/LoaderInput.story.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/branded/src/components/LoaderInput.story.tsx b/client/branded/src/components/LoaderInput.story.tsx index e04e91dfcce3..8db4829b5154 100644 --- a/client/branded/src/components/LoaderInput.story.tsx +++ b/client/branded/src/components/LoaderInput.story.tsx @@ -3,6 +3,7 @@ import { storiesOf } from '@storybook/react' import React from 'react' import { LoaderInput } from './LoaderInput' import { BrandedStory } from './BrandedStory' +import webStyles from '../../../web/src/main.scss' const { add } = storiesOf('branded/LoaderInput', module).addDecorator(story => (
    @@ -11,7 +12,7 @@ const { add } = storiesOf('branded/LoaderInput', module).addDecorator(story => ( )) add('Interactive', () => ( - + {() => ( From edabd7c9f51d26630d100c3a373cf87d288978ae Mon Sep 17 00:00:00 2001 From: Artem Ruts <1319181+artemruts@users.noreply.github.com> Date: Mon, 30 Nov 2020 16:41:08 -0500 Subject: [PATCH 20/22] Attempt to cast types --- .../src/util/useInputValidation.test.ts | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/client/shared/src/util/useInputValidation.test.ts b/client/shared/src/util/useInputValidation.test.ts index 9171c6f09317..bbe24977d3ac 100644 --- a/client/shared/src/util/useInputValidation.test.ts +++ b/client/shared/src/util/useInputValidation.test.ts @@ -1,8 +1,11 @@ +/* eslint-disable @typescript-eslint/no-floating-promises */ +import { renderHook, act } from '@testing-library/react-hooks' import { min, noop } from 'lodash' import { Observable, of, Subject, Subscription } from 'rxjs' import { delay } from 'rxjs/operators' import * as sinon from 'sinon' import { + useInputValidation, createValidationPipeline, InputValidationState, ValidationOptions, @@ -311,4 +314,41 @@ describe('useInputValidation()', () => { expect(executeUserInputScript(inputs)).toStrictEqual(expectedStates) }) + + it('works with the state override', () => { + const { result } = renderHook(() => + useInputValidation({ + synchronousValidators: [isDotCo], + }) + ) + + act(() => { + const [, nextEmailFieldChange, emailInputReference] = result.current + const inputElement = createEmailInputElement() + emailInputReference.current = (inputElement as unknown) as HTMLInputElement + + inputElement.changeValue('test-string') + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + nextEmailFieldChange({ + target: emailInputReference.current, + preventDefault: noop, + } as React.ChangeEvent) + }) + + expect(result.current[0]).toStrictEqual({ value: 'test-string', kind: 'LOADING' }) + + act(() => { + const overrideEmailState = result.current[3] + overrideEmailState({ value: 'test@sg.co', validate: false }) + }) + + expect(result.current[0]).toStrictEqual({ value: 'test@sg.co', kind: 'NOT_VALIDATED' }) + + act(() => { + const overrideEmailState = result.current[3] + overrideEmailState({ value: '' }) + }) + + expect(result.current[0]).toStrictEqual({ value: '', kind: 'NOT_VALIDATED' }) + }) }) From 6b6aa0947c9a6fc512610092bd062870b048a992 Mon Sep 17 00:00:00 2001 From: Artem Ruts <1319181+artemruts@users.noreply.github.com> Date: Tue, 1 Dec 2020 10:39:11 -0500 Subject: [PATCH 21/22] Update client/shared/src/util/useInputValidation.test.ts Co-authored-by: Felix Becker --- client/shared/src/util/useInputValidation.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/client/shared/src/util/useInputValidation.test.ts b/client/shared/src/util/useInputValidation.test.ts index bbe24977d3ac..790df4e7a265 100644 --- a/client/shared/src/util/useInputValidation.test.ts +++ b/client/shared/src/util/useInputValidation.test.ts @@ -328,7 +328,6 @@ describe('useInputValidation()', () => { emailInputReference.current = (inputElement as unknown) as HTMLInputElement inputElement.changeValue('test-string') - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions nextEmailFieldChange({ target: emailInputReference.current, preventDefault: noop, From 8760033c7013cf96ade524b6c1b05e9dfd0c7b47 Mon Sep 17 00:00:00 2001 From: Artem Ruts <1319181+artemruts@users.noreply.github.com> Date: Tue, 1 Dec 2020 12:32:51 -0500 Subject: [PATCH 22/22] Addressing feedback --- .../emails/SetUserPrimaryEmailForm.tsx | 8 +--- .../src/user/settings/emails/UserEmail.tsx | 17 +++---- .../emails/UserSettingsEmailsPage.tsx | 48 +++++++++---------- 3 files changed, 32 insertions(+), 41 deletions(-) diff --git a/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx index ef94d1b616e8..dbeed52a8e7d 100644 --- a/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx +++ b/client/web/src/user/settings/emails/SetUserPrimaryEmailForm.tsx @@ -32,12 +32,8 @@ export const SetUserPrimaryEmailForm: FunctionComponent = ({ user, emails const [primaryEmail, setPrimaryEmail] = useState(findPrimaryEmail(emails)) const [statusOrError, setStatusOrError] = useState() - const options = emails.reduce((accumulator: string[], email) => { - if (email.verified) { - accumulator.push(email.email) - } - return accumulator - }, []) + // options should include all verified emails + a primary one + const options = emails.filter(email => email.verified || email.isPrimary).map(email => email.email) const onPrimaryEmailSelect: React.ChangeEventHandler = event => setPrimaryEmail(event.target.value) diff --git a/client/web/src/user/settings/emails/UserEmail.tsx b/client/web/src/user/settings/emails/UserEmail.tsx index cc1e18e83a57..eeccf01e655d 100644 --- a/client/web/src/user/settings/emails/UserEmail.tsx +++ b/client/web/src/user/settings/emails/UserEmail.tsx @@ -96,21 +96,18 @@ export const UserEmail: FunctionComponent = ({ } } - let verifiedLinkFragment: React.ReactFragment - if (verified) { - verifiedLinkFragment = Verified - } else if (verificationPending) { - verifiedLinkFragment = Verification pending - } else { - verifiedLinkFragment = Not verified - } - return ( <>
    {email} - {verifiedLinkFragment} + {verified ? ( + Verified + ) : verificationPending ? ( + Verification pending + ) : ( + Not verified + )} {isPrimary && Primary}
    diff --git a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx index ada92cb8556a..cb39c63f63f0 100644 --- a/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx +++ b/client/web/src/user/settings/emails/UserSettingsEmailsPage.tsx @@ -36,34 +36,32 @@ export const UserSettingsEmailsPage: FunctionComponent = ({ user, history const fetchEmails = useCallback(async (): Promise => { setStatusOrError('loading') - try { - const fetchedEmails = dataOrThrowErrors( - await requestGraphQL( - gql` - query UserEmails($user: ID!) { - node(id: $user) { - ... on User { - emails { - email - isPrimary - verified - verificationPending - viewerCanManuallyVerify - } + const fetchedEmails = dataOrThrowErrors( + await requestGraphQL( + gql` + query UserEmails($user: ID!) { + node(id: $user) { + ... on User { + emails { + email + isPrimary + verified + verificationPending + viewerCanManuallyVerify } } } - `, - { user: user.id } - ).toPromise() - ) - - if (fetchedEmails?.node?.emails) { - setEmails(fetchedEmails.node.emails) - setStatusOrError('loaded') - } - } catch (error) { - setStatusOrError(asError(error)) + } + `, + { user: user.id } + ).toPromise() + ) + + if (fetchedEmails?.node?.emails) { + setEmails(fetchedEmails.node.emails) + setStatusOrError('loaded') + } else { + setStatusOrError(asError("Sorry, we couldn't fetch user emails. Try again?")) } }, [user, setStatusOrError, setEmails])