Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update account settings page #16

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

yasell
Copy link

@yasell yasell commented Jan 20, 2025

This PR updates the profile-settings page according to the design:

General Account Settings

2

Keys and Tokens

4

Create a token modal

6

Delete a token modal

5

Copy link

vercel bot commented Jan 20, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
canary ❌ Failed (Inspect) Jan 21, 2025 2:20pm

@@ -74,18 +73,17 @@ const SettingsAccountGeneralPage: React.FC<SettingsAccountGeneralPageProps> = ({
onUpdatePassword,
useTranslationStore
}) => {
// Profile form handling
const { t } = useTranslationStore()
const { userData } = useProfileSettingsStore()
const [profileSubmitted, setProfileSubmitted] = useState(false)
const [passwordSubmitted, setPasswordSubmitted] = useState(false)
const TRUNCATE_INITIALS_LEN = 2
Copy link

Choose a reason for hiding this comment

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

don't need it, can use getInitials without second prop

const onPasswordSubmit: SubmitHandler<PasswordFields> = data => {
onUpdatePassword(data)
}

if (isLoadingUser) {
return (
<SandboxLayout.Main>
<SandboxLayout.Content maxWidth="2xl">
<SandboxLayout.Content>
<SkeletonList />
Copy link

Choose a reason for hiding this comment

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

For loading forms we have SkeletonForm

</Avatar>
</ControlGroup>
<SandboxLayout.Content className="max-w-[476px] px-0">
<Text size={5} weight={'medium'} as="h1">
Copy link

Choose a reason for hiding this comment

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

we should avoid using Text component - in future we will remove it at all, lets use h1 here

<Avatar size="80" className="size-20 rounded-full bg-primary/[0.02] shadow-md">
<AvatarImage src="/images/anon.jpg" />
<AvatarFallback>
<Text size={5} weight="medium" color="tertiaryBackground">
Copy link

Choose a reason for hiding this comment

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

we should avoid using Text component - use span here

<FormWrapper onSubmit={handleProfileSubmit(onProfileSubmit)}>
<Legend title={t('views:profileSettings.personalInfo', 'Personal information')} />
<ControlGroup>
<Avatar size="80" className="size-20 rounded-full bg-primary/[0.02] shadow-md">
Copy link

Choose a reason for hiding this comment

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

we don't need all this classes, size use with separate prop size

placeholder={t('views:profileSettings.enterNamePlaceholder', 'Enter your name')}
label={t('views:profileSettings.name', 'Name')}
/>
{profileErrors.name && (
Copy link

Choose a reason for hiding this comment

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

In the Input component, we have a special error prop for error messages, so this part of the code is not needed.

disabled
label={t('views:profileSettings.username', 'Username')}
/>
<Message className="mt-1.5" theme={MessageTheme.DEFAULT}>
Copy link

Choose a reason for hiding this comment

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

we have caption prop at Input component for input's description text

@@ -51,6 +49,7 @@ export const SettingsProfileKeysPage = () => {
const [isAlertDeleteDialogOpen, setIsAlertDeleteDialogOpen] = useState(false)
const [alertParams, setAlertParams] = useState<AlertDeleteParams | null>(null)
const [searchParams] = useSearchParams()
const CONVERT_DAYS_TO_NANO_SECONDS = 24 * 60 * 60 * 1000 * 1000000
Copy link

Choose a reason for hiding this comment

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

Move the static variable out of the component.

description={t(
'views:profileSettings.addTokenDescription',
'Personal access tokens allow you to authenticate with the API.'
<SandboxLayout.Content className="max-w-[812px] px-0">
Copy link

Choose a reason for hiding this comment

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

don't need all classes

<Fieldset className="gap-y-5">
<Legend
title={
<span className="flex justify-between">
Copy link

Choose a reason for hiding this comment

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

should add some gap-x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants