Skip to content

Commit 6725bc5

Browse files
hiderrhiderr
committed
code review fixes
1 parent 0225743 commit 6725bc5

File tree

13 files changed

+105
-84
lines changed

13 files changed

+105
-84
lines changed

‎apps/gitness/src/components-v2/create-branch-dialog.tsx

+11-23
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import { useCallback, useEffect, useState } from 'react'
1+
import { useCallback, useState } from 'react'
22

33
import { useCreateBranchMutation } from '@harnessio/code-service-client'
4-
import { useToast } from '@harnessio/ui/components'
4+
import { useToastNotification } from '@harnessio/ui/components'
55
import {
66
BranchSelectorListItem,
77
CreateBranchDialog as CreateBranchDialogComp,
@@ -15,38 +15,25 @@ import { BranchSelectorContainer } from './branch-selector-container'
1515
interface CreateBranchDialogProps {
1616
open: boolean
1717
onClose: () => void
18+
onSuccess?: () => void
1819
}
1920

20-
export const CreateBranchDialog = ({ open, onClose }: CreateBranchDialogProps) => {
21+
export const CreateBranchDialog = ({ open, onClose, onSuccess }: CreateBranchDialogProps) => {
2122
const repo_ref = useGetRepoRef()
22-
const { toast } = useToast()
23+
2324
const { t } = useTranslationStore()
2425
const [selectedBranchOrTag, setSelectedBranchOrTag] = useState<BranchSelectorListItem | null>(null)
2526

26-
const [showToast, setShowToast] = useState(false)
27-
const [toastId, setToastId] = useState<string | null>(null)
28-
2927
const [createdBranchName, setCreatedBranchName] = useState<string>('')
3028

3129
const selectBranchOrTag = useCallback((branchTagName: BranchSelectorListItem) => {
3230
setSelectedBranchOrTag(branchTagName)
3331
}, [])
3432

35-
useEffect(() => {
36-
if (!open) {
37-
setShowToast(false)
38-
setToastId(null)
39-
}
40-
41-
if (showToast && !toastId) {
42-
const { id } = toast({
43-
title: t('views:repos.branchCreated'),
44-
description: t('views:repos.branchCreatedDescription', { name: createdBranchName })
45-
})
46-
47-
setToastId(id)
48-
}
49-
}, [createdBranchName, showToast, t, toast, toastId, open])
33+
const { showToast } = useToastNotification({
34+
title: t('views:repos.branchCreated'),
35+
description: t('views:repos.branchCreatedDescription', { name: createdBranchName })
36+
})
5037

5138
const {
5239
mutateAsync: createBranch,
@@ -58,7 +45,8 @@ export const CreateBranchDialog = ({ open, onClose }: CreateBranchDialogProps) =
5845
onSuccess: data => {
5946
onClose()
6047
setCreatedBranchName(data.body.name ?? '')
61-
setShowToast(true)
48+
showToast()
49+
onSuccess?.()
6250
}
6351
}
6452
)

‎apps/gitness/src/pages-v2/repo/repo-branch-list.tsx

+5-1
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,11 @@ export function RepoBranchesListPage() {
192192
setCreateBranchSearchQuery={setCreateBranchSearchQuery}
193193
/>
194194

195-
<CreateBranchDialog open={isCreateBranchDialogOpen} onClose={() => setCreateBranchDialogOpen(false)} />
195+
<CreateBranchDialog
196+
open={isCreateBranchDialogOpen}
197+
onClose={() => setCreateBranchDialogOpen(false)}
198+
onSuccess={handleInvalidateBranchList}
199+
/>
196200

197201
<DeleteAlertDialog
198202
open={deleteBranchName !== null}

‎apps/gitness/src/pages-v2/repo/repo-tags-list-container.tsx

+3-6
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,9 @@ export const RepoTagsListContainer = () => {
116116
})
117117
}
118118

119-
const selectBranchOrTag = useCallback(
120-
(branchTagName: BranchSelectorListItem) => {
121-
setSelectedBranchOrTag(branchTagName)
122-
},
123-
[repoId, spaceId]
124-
)
119+
const selectBranchOrTag = useCallback((branchTagName: BranchSelectorListItem) => {
120+
setSelectedBranchOrTag(branchTagName)
121+
}, [])
125122

126123
return (
127124
<>

‎packages/ui/locales/en/views.json

+3-4
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,9 @@
113113
"createBranchDialog": {
114114
"validation": {
115115
"name": "Branch name is required",
116-
"nameRegex": "Name must contain only letters, numbers, and the characters: - _ .",
117116
"noSpaces": "Name cannot contain spaces",
118-
"target": "Base branch is required"
117+
"target": "Base branch is required",
118+
"nameRegex": "Name must contain only letters, numbers, and the characters: - _ ."
119119
}
120120
},
121121
"createBranchTitle": "Create a branch",
@@ -171,10 +171,9 @@
171171
"createTagDialog": {
172172
"validation": {
173173
"name": "Tag name is required",
174-
"nameMax": "Name must be no longer than 40 characters",
175-
"nameRegex": "Name must contain only letters, numbers, and the characters: - _ .",
176174
"noSpaces": "Name cannot contain spaces",
177175
"target": "Base branch is required",
176+
"nameRegex": "Name must contain only letters, numbers, and the characters: - _ .",
178177
"message": "Description is required"
179178
},
180179
"name": "Name",

‎packages/ui/locales/fr/views.json

+3-4
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,9 @@
113113
"createBranchDialog": {
114114
"validation": {
115115
"name": "Le nom de la branche est requis",
116-
"nameRegex": "Le nom ne doit contenir que des lettres, des chiffres et les caractères : - _ .",
117116
"noSpaces": "Le nom ne peut pas contenir d'espaces",
118-
"target": "La branche de base est requise"
117+
"target": "La branche de base est requise",
118+
"nameRegex": "Le nom ne doit contenir que des lettres, des chiffres et les caractères : - _ ."
119119
}
120120
},
121121
"createBranchTitle": "Créer une branche",
@@ -171,10 +171,9 @@
171171
"createTagDialog": {
172172
"validation": {
173173
"name": "Le nom du tag est requis",
174-
"nameMax": "Le nom ne doit pas dépasser 40 caractères",
175-
"nameRegex": "Le nom ne doit contenir que des lettres, des chiffres et les caractères : - _ .",
176174
"noSpaces": "Le nom ne peut pas contenir d'espaces",
177175
"target": "La branche de base est requise",
176+
"nameRegex": "Le nom ne doit contenir que des lettres, des chiffres et les caractères : - _ .",
178177
"message": "La description est requise"
179178
},
180179
"name": "Nom",

‎packages/ui/src/components/alert/AlertTitle.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export interface AlertTitleProps extends PropsWithChildren<React.HTMLAttributes<
77
}
88

99
export const AlertTitle = forwardRef<HTMLHeadingElement, AlertTitleProps>(({ className, children }, ref) => (
10-
<h5 ref={ref} className={cn('mb-1 font-medium leading-none tracking-tight', className)}>
10+
<h5 ref={ref} className={cn('mb-1 font-medium leading-none tracking-tight break-words', className)}>
1111
{children}
1212
</h5>
1313
))

‎packages/ui/src/components/commit-copy-actions.tsx

+4-4
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ import copy from 'clipboard-copy'
77
export const CommitCopyActions = ({
88
sha,
99
toCommitDetails,
10-
rootClassName
10+
className
1111
}: {
1212
sha: string
1313
toCommitDetails?: ({ sha }: { sha: string }) => string
14-
rootClassName?: string
14+
className?: string
1515
}) => {
1616
const [copied, setCopied] = useState(false)
1717
const { navigate } = useRouterContext()
@@ -32,10 +32,10 @@ export const CommitCopyActions = ({
3232
}
3333

3434
return (
35-
<ShaBadge.Root className={rootClassName}>
35+
<ShaBadge.Root className={className}>
3636
<ShaBadge.Content className="p-0" asChild>
3737
<button
38-
className="size-full w-[67px] px-2 text-13 text-foreground-3"
38+
className="text-13 text-foreground-3 size-full w-[67px] px-2"
3939
onClick={() => handleNavigation()}
4040
onKeyDown={e => {
4141
if (e.key === 'Enter' || e.key === ' ') handleNavigation()

‎packages/ui/src/components/toast/use-toast.ts

+35-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { ReactNode, useEffect, useState } from 'react'
1+
import { ReactNode, useCallback, useEffect, useState } from 'react'
22

33
import type { ToastActionElement, ToastProps } from './toast'
44

@@ -150,3 +150,37 @@ function useToast() {
150150
}
151151

152152
export { useToast, toast }
153+
154+
interface UseToastNotificationProps {
155+
title: string
156+
description: string
157+
duration?: number
158+
action?: ToastActionElement
159+
}
160+
161+
export function useToastNotification({ title, description, duration, action }: UseToastNotificationProps) {
162+
const { toast } = useToast()
163+
164+
const [show, setShow] = useState(false)
165+
166+
useEffect(() => {
167+
if (show) {
168+
toast({
169+
title,
170+
description,
171+
duration,
172+
action
173+
})
174+
175+
setShow(false)
176+
}
177+
}, [title, description, toast, show, duration, action])
178+
179+
const showToast = useCallback(() => {
180+
setShow(true)
181+
}, [setShow])
182+
183+
return {
184+
showToast
185+
}
186+
}

‎packages/ui/src/views/repo/repo-branch/components/create-branch-dialog.tsx

+2-8
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,6 @@ export const createBranchFormSchema = (t: TranslationStore['t']) =>
1414
.string()
1515
.trim()
1616
.min(1, { message: t('views:repos.createBranchDialog.validation.name', 'Branch name is required') })
17-
.regex(/^[a-zA-Z0-9._-\s]+$/, {
18-
message: t(
19-
'views:repos.createBranchDialog.validation.nameRegex',
20-
'Name must contain only letters, numbers, and the characters: - _ .'
21-
)
22-
})
2317
.refine(data => !data.includes(' '), {
2418
message: t('views:repos.createBranchDialog.validation.noSpaces', 'Name cannot contain spaces')
2519
}),
@@ -80,7 +74,7 @@ export function CreateBranchDialog({
8074

8175
return (
8276
<Dialog.Root open={open} onOpenChange={handleClose}>
83-
<Dialog.Content className="max-w-[460px] border-border bg-background-1" aria-describedby={undefined}>
77+
<Dialog.Content className="border-border bg-background-1 max-w-[460px]" aria-describedby={undefined}>
8478
<Dialog.Header>
8579
<Dialog.Title className="font-medium">{t('views:repos.createBranchTitle', 'Create a branch')}</Dialog.Title>
8680
</Dialog.Header>
@@ -90,7 +84,7 @@ export function CreateBranchDialog({
9084
id="name"
9185
label="Branch name"
9286
{...register('name')}
93-
maxLength={50}
87+
maxLength={250}
9488
placeholder={t('views:forms.enterBranchName', 'Enter branch name')}
9589
size="md"
9690
error={errors.name?.message}

‎packages/ui/src/views/repo/repo-tags/components/create-tag/create-tag-dialog.tsx

+9-4
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,14 @@ export const CreateTagDialog: FC<CreateTagDialogProps> = ({
5454
}, [clearErrors, reset])
5555

5656
useEffect(() => {
57-
if (selectedBranchOrTag) {
58-
setValue('target', selectedBranchOrTag.name, { shouldValidate: true })
57+
if (open) {
58+
resetForm()
59+
60+
if (selectedBranchOrTag) {
61+
setValue('target', selectedBranchOrTag.name, { shouldValidate: true })
62+
}
5963
}
60-
}, [selectedBranchOrTag, setValue])
64+
}, [open, resetForm, selectedBranchOrTag, setValue])
6165

6266
const handleClose = () => {
6367
resetForm()
@@ -66,7 +70,7 @@ export const CreateTagDialog: FC<CreateTagDialogProps> = ({
6670

6771
return (
6872
<Dialog.Root open={open} onOpenChange={handleClose}>
69-
<Dialog.Content className="max-w-xl border-border bg-background-1" aria-describedby={undefined}>
73+
<Dialog.Content className="border-border bg-background-1 max-w-xl" aria-describedby={undefined}>
7074
<Dialog.Header>
7175
<Dialog.Title className="font-medium">{t('views:repos.createTagTitle', 'Create a tag')}</Dialog.Title>
7276
</Dialog.Header>
@@ -77,6 +81,7 @@ export const CreateTagDialog: FC<CreateTagDialogProps> = ({
7781
id="name"
7882
label={t('views:forms.tagName', 'Name')}
7983
{...register('name')}
84+
maxLength={250}
8085
placeholder={t('views:forms.enterTagName', 'Enter a tag name here')}
8186
size="md"
8287
error={errors.name?.message}

‎packages/ui/src/views/repo/repo-tags/components/create-tag/schema.ts

-9
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,6 @@ export const makeCreateTagFormSchema = (t: TranslationStore['t']) =>
77
.string()
88
.trim()
99
.min(1, { message: t('views:repos.createTagDialog.validation.name', 'Tag name is required') })
10-
.max(40, {
11-
message: t('views:repos.createTagDialog.validation.nameMax', 'Name must be no longer than 40 characters')
12-
})
13-
.regex(/^[a-zA-Z0-9._-\s]+$/, {
14-
message: t(
15-
'views:repos.createTagDialog.validation.nameRegex',
16-
'Name must contain only letters, numbers, and the characters: - _ .'
17-
)
18-
})
1910
.refine(data => !data.includes(' '), {
2011
message: t('views:repos.createTagDialog.validation.noSpaces', 'Name cannot contain spaces')
2112
}),

‎packages/ui/src/views/repo/repo-tags/components/repo-tags-list.tsx

+21-17
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export const RepoTagsList: React.FC<RepoTagsListProps> = ({
6060

6161
if (!isLoading && !tagsList?.length) {
6262
return (
63-
<div className={cn(!isDirtyList && 'flex items-center justify-center py-[232px]')}>
63+
<div className={cn(!isDirtyList && 'flex items-center justify-center py-[204px]')}>
6464
<NoData
6565
iconName={isDirtyList ? 'no-search-magnifying-glass' : 'no-data-tags'}
6666
withBorder={isDirtyList}
@@ -98,15 +98,15 @@ export const RepoTagsList: React.FC<RepoTagsListProps> = ({
9898
}
9999

100100
return (
101-
<Table.Root variant="asStackedList">
101+
<Table.Root variant="asStackedList" tableClassName="table-fixed">
102102
<Table.Header>
103103
<Table.Row className="pointer-events-none select-none">
104-
<Table.Head className="w-[124px]">{t('views:repos.tag', 'Tag')}</Table.Head>
105-
<Table.Head className="w-[330px]">{t('views:repos.description', 'Description')}</Table.Head>
106-
<Table.Head className="w-[150px]">{t('views:repos.commit', 'Commit')}</Table.Head>
107-
<Table.Head className="w-[178px]">{t('views:repos.tagger', 'Tagger')}</Table.Head>
108-
<Table.Head className="w-[130px]">{t('views:repos.creationDate', 'Creation date')}</Table.Head>
109-
<Table.Head className="w-[48px]" />
104+
<Table.Head className="w-[11%]">{t('views:repos.tag', 'Tag')}</Table.Head>
105+
<Table.Head className="w-[35%]">{t('views:repos.description', 'Description')}</Table.Head>
106+
<Table.Head className="w-[16%] min-w-[128px]">{t('views:repos.commit', 'Commit')}</Table.Head>
107+
<Table.Head className="w-[17%]">{t('views:repos.tagger', 'Tagger')}</Table.Head>
108+
<Table.Head className="w-[16%]">{t('views:repos.creationDate', 'Creation date')}</Table.Head>
109+
<Table.Head className="w-[5%] min-w-[32px]" />
110110
</Table.Row>
111111
</Table.Header>
112112

@@ -115,27 +115,31 @@ export const RepoTagsList: React.FC<RepoTagsListProps> = ({
115115
) : (
116116
<Table.Body hasHighlightOnHover>
117117
{tagsList.map(tag => (
118-
<Table.Row key={tag.sha}>
119-
<Table.Cell className="max-w-[124px] truncate !py-[13px] text-foreground-1">{tag.name}</Table.Cell>
120-
<Table.Cell className="max-w-[330px] !py-[13px] text-foreground-3">
121-
<div className="line-clamp-3 break-all">{tag.message}</div>
118+
<Table.Row key={tag.sha} className="min-h-[48px]">
119+
<Table.Cell className="text-foreground-1 w-[11%] truncate !pb-3 !pt-4">
120+
<div className="h-4 overflow-hidden truncate">{tag.name}</div>
122121
</Table.Cell>
123-
<Table.Cell className="!py-2.5 text-foreground-3">
122+
<Table.Cell className="text-foreground-3 w-[35%] !py-4">
123+
<div className="line-clamp-3 overflow-hidden break-all leading-4">{tag.message}</div>
124+
</Table.Cell>
125+
<Table.Cell className="text-foreground-3 w-[16%] min-w-[128px] !py-2.5">
124126
<div className="flex">
125-
<CommitCopyActions sha={tag.sha} toCommitDetails={toCommitDetails} rootClassName="h-7" />
127+
<CommitCopyActions sha={tag.sha} toCommitDetails={toCommitDetails} className="h-7" />
126128
</div>
127129
</Table.Cell>
128-
<Table.Cell className="!py-[13px] text-foreground-3">
130+
<Table.Cell className="text-foreground-3 w-[17%] !pb-3 !pt-4">
129131
<div className="flex items-center gap-2">
130132
<Avatar.Root size="4.5" className="rounded-full text-white">
131133
<Avatar.Fallback>{getInitials(tag.tagger?.identity.name || '')}</Avatar.Fallback>
132134
</Avatar.Root>
133135
<span>{tag.tagger?.identity.name}</span>
134136
</div>
135137
</Table.Cell>
136-
<Table.Cell className="!py-[13px] text-foreground-3">{getCreationDate(tag)}</Table.Cell>
138+
<Table.Cell className="text-foreground-3 w-[16%] !pb-3 !pt-4">
139+
<div>{getCreationDate(tag)}</div>
140+
</Table.Cell>
137141

138-
<Table.Cell className="w-[54px] !py-2.5 text-right">
142+
<Table.Cell className="w-[5%] !pb-1.5 !pt-2.5 text-right">
139143
<MoreActionsTooltip
140144
isInTable
141145
actions={getTableActions(tag).map(action => ({

‎packages/ui/src/views/repo/repo-tags/repo-tags-list-page.tsx

+8-2
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,16 @@ export const RepoTagsListView: FC<RepoTagsListViewProps> = ({
3636
}, [page, searchQuery])
3737

3838
return (
39-
<SandboxLayout.Main className="max-w-[1000px]">
40-
<SandboxLayout.Content className={cn({ 'h-full': !isLoading && !tagsList.length && !searchQuery })}>
39+
<SandboxLayout.Main>
40+
<SandboxLayout.Content
41+
className={cn({
42+
'max-w-[1000px] mx-auto': isLoading || tagsList.length || searchQuery,
43+
'h-full': !isLoading && !tagsList.length && !searchQuery
44+
})}
45+
>
4146
{(isLoading || !!tagsList.length || isDirtyList) && (
4247
<>
48+
<Spacer size={2} />
4349
<span className="text-24 text-foreground-1 font-medium">{t('views:repos.tags', 'Tags')}</span>
4450
<Spacer size={6} />
4551
<ListActions.Root>

0 commit comments

Comments
 (0)