From d5c3f06751d3d32707c906cc6d0bfc8846ab411b Mon Sep 17 00:00:00 2001 From: Ilya Topilskii Date: Thu, 26 Dec 2024 15:44:43 +0100 Subject: [PATCH] feat: pr-compare/create design improvements (#578) * refactor: refactoring pull-request-compare-page fix: minor changes fix: minor changes fix: minor changes fix: minor changes fix: minor changes fix: minor changes * fix: minor changes * fix: minor changes --------- Co-authored-by: Andrew Golovanov --- .../pull-request-compare.tsx | 6 +- .../pull-request/pull-request-compare.tsx | 4 +- packages/ui/src/components/button.tsx | 1 + packages/ui/src/components/icon.tsx | 6 +- packages/ui/src/components/no-data.tsx | 1 + packages/ui/src/icons/compare.svg | 15 + packages/ui/src/icons/no-data-pr.svg | 60 +++ packages/ui/src/icons/page-state.svg | 60 +++ packages/ui/src/views/index.ts | 1 - .../layouts/PullRequestCompareLayout.tsx | 450 ------------------ .../commit-selector/commit-selector.tsx | 3 +- .../repo/components/commit-selector/types.ts | 3 +- .../components/commits-list.tsx | 0 .../commits/pull-request-commits.types.ts | 2 +- .../pull-request-compare-button.tsx | 20 +- .../pull-request-compare-diff-list.tsx | 143 ++++++ .../components/pull-request-compare-form.tsx | 12 +- .../pull-request-compare-tab-trigger-item.tsx | 6 +- .../pull-request/compare/components/types.ts | 4 - .../compare/pull-request-compare-page.tsx | 356 ++++++++++++++ .../compare/pull-request-compare.types.ts | 11 + .../pull-request-diff-viewer.tsx | 2 +- .../pull-request-reviewers-header.tsx | 0 .../pull-request-reviewers-item.tsx | 3 +- .../pull-request-reviewers-list.tsx | 2 +- .../pull-request-reviewers-tooltip.tsx | 2 +- .../pull-request-side-bar.tsx | 2 +- .../{diff-viewer => }/constants.ts | 0 .../changes/pull-request-changes-filter.tsx | 10 +- .../changes/pull-request-changes.tsx | 2 +- .../conversation/pull-request-overview.tsx | 2 +- .../details/pull-request-changes-page.tsx | 8 +- .../details/pull-request-details-types.ts | 19 +- .../details/pull-request-utils.ts | 2 +- .../ui/src/views/repo/pull-request/index.ts | 6 +- .../repo/pull-request/pull-request.types.ts | 62 +-- .../ui/src/views/repo/repo-commits/index.tsx | 2 +- .../repo/repo-commits/repo-commits-view.tsx | 5 +- .../ui/src/views/repo/repo-commits/types.ts | 40 +- packages/ui/src/views/repo/repo.types.ts | 39 ++ 40 files changed, 768 insertions(+), 604 deletions(-) create mode 100644 packages/ui/src/icons/compare.svg create mode 100644 packages/ui/src/icons/no-data-pr.svg create mode 100644 packages/ui/src/icons/page-state.svg delete mode 100644 packages/ui/src/views/layouts/PullRequestCompareLayout.tsx rename packages/ui/src/views/repo/{repo-commits => }/components/commits-list.tsx (100%) create mode 100644 packages/ui/src/views/repo/pull-request/compare/components/pull-request-compare-diff-list.tsx delete mode 100644 packages/ui/src/views/repo/pull-request/compare/components/types.ts create mode 100644 packages/ui/src/views/repo/pull-request/compare/pull-request-compare-page.tsx create mode 100644 packages/ui/src/views/repo/pull-request/compare/pull-request-compare.types.ts rename packages/ui/src/views/repo/pull-request/{diff-viewer => components}/pull-request-diff-viewer.tsx (99%) rename packages/ui/src/views/repo/pull-request/{details/components/conversation => components}/pull-request-reviewers-header.tsx (100%) rename packages/ui/src/views/repo/pull-request/{details/components/conversation => components}/pull-request-reviewers-item.tsx (92%) rename packages/ui/src/views/repo/pull-request/{details/components/conversation => components}/pull-request-reviewers-list.tsx (97%) rename packages/ui/src/views/repo/pull-request/{details/components/conversation => components}/pull-request-reviewers-tooltip.tsx (94%) rename packages/ui/src/views/repo/pull-request/{details/components/conversation => components}/pull-request-side-bar.tsx (97%) rename packages/ui/src/views/repo/pull-request/{diff-viewer => }/constants.ts (100%) diff --git a/apps/design-system/src/views/pull-request-compare/pull-request-compare.tsx b/apps/design-system/src/views/pull-request-compare/pull-request-compare.tsx index 874c62b66..05da975a3 100644 --- a/apps/design-system/src/views/pull-request-compare/pull-request-compare.tsx +++ b/apps/design-system/src/views/pull-request-compare/pull-request-compare.tsx @@ -1,12 +1,12 @@ import { FC, useCallback } from 'react' -import { PullRequestCompare, SandboxPullRequestCompareProps } from '@harnessio/ui/views' +import { PullRequestComparePage, PullRequestComparePageProps } from '@harnessio/ui/views' import { noop, useTranslationsStore } from '../../utils.ts' import { repoBranchListStore } from './repo-branch-store.ts' import { repoCommitStore } from './repo-commit-store.ts' -const PullRequestCompareWrapper: FC> = props => { +const PullRequestCompareWrapper: FC> = props => { const useRepoBranchListStore = useCallback( () => ({ ...repoBranchListStore, @@ -23,7 +23,7 @@ const PullRequestCompareWrapper: FC> = p ) return ( - { if (isFetchingBranches) return return ( - >> diff --git a/packages/ui/src/components/no-data.tsx b/packages/ui/src/components/no-data.tsx index 16c2f7bda..4600dd740 100644 --- a/packages/ui/src/components/no-data.tsx +++ b/packages/ui/src/components/no-data.tsx @@ -21,6 +21,7 @@ export interface NoDataProps { | 'no-repository' | 'no-data-error' | 'no-data-commits' + | 'no-data-pr' iconSize?: number description: string[] primaryButton?: { diff --git a/packages/ui/src/icons/compare.svg b/packages/ui/src/icons/compare.svg new file mode 100644 index 000000000..40b2e6060 --- /dev/null +++ b/packages/ui/src/icons/compare.svg @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + diff --git a/packages/ui/src/icons/no-data-pr.svg b/packages/ui/src/icons/no-data-pr.svg new file mode 100644 index 000000000..6f27256f1 --- /dev/null +++ b/packages/ui/src/icons/no-data-pr.svg @@ -0,0 +1,60 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/packages/ui/src/icons/page-state.svg b/packages/ui/src/icons/page-state.svg new file mode 100644 index 000000000..371648256 --- /dev/null +++ b/packages/ui/src/icons/page-state.svg @@ -0,0 +1,60 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/packages/ui/src/views/index.ts b/packages/ui/src/views/index.ts index f0a9b4674..7e0d52be7 100644 --- a/packages/ui/src/views/index.ts +++ b/packages/ui/src/views/index.ts @@ -5,7 +5,6 @@ export * from './types/link-types' export * as SandboxLayout from './layouts/SandboxLayout' export * from './layouts/SandboxRoot' export * from './layouts/Floating1ColumnLayout' -export * from './layouts/PullRequestCompareLayout' export * from './layouts/PullRequestLayout' // Account diff --git a/packages/ui/src/views/layouts/PullRequestCompareLayout.tsx b/packages/ui/src/views/layouts/PullRequestCompareLayout.tsx deleted file mode 100644 index a10d69baa..000000000 --- a/packages/ui/src/views/layouts/PullRequestCompareLayout.tsx +++ /dev/null @@ -1,450 +0,0 @@ -import { useEffect, useRef, useState } from 'react' -import { useForm } from 'react-hook-form' -import { useNavigate } from 'react-router-dom' - -import { TypesDiffStats } from '@/types' -import { Accordion, AccordionContent, AccordionItem, AccordionTrigger } from '@components/accordion' -import { - Button, - CopyButton, - Icon, - ListActions, - NoData, - Spacer, - StackedList, - Tabs, - TabsContent, - TabsList, - Text -} from '@components/index' -import { DiffModeEnum } from '@git-diff-view/react' -import { zodResolver } from '@hookform/resolvers/zod' -import { BranchSelector } from '@views/repo/components/branch-selector/branch-selector' -import { ICommitSelectorStore } from '@views/repo/components/commit-selector/types' -import PullRequestCompareButton from '@views/repo/pull-request/compare/components/pull-request-compare-button' -import PullRequestCompareForm from '@views/repo/pull-request/compare/components/pull-request-compare-form' -import TabTriggerItem from '@views/repo/pull-request/compare/components/pull-request-compare-tab-trigger-item' -import { CommitSelectorListItem } from '@views/repo/pull-request/compare/components/types' -import PullRequestDiffViewer from '@views/repo/pull-request/diff-viewer/pull-request-diff-viewer' -import { useDiffConfig } from '@views/repo/pull-request/hooks/useDiffConfig' -import { parseStartingLineIfOne } from '@views/repo/pull-request/utils' -import { z } from 'zod' - -import { - BranchSelectorListItem, - BranchSelectorTab, - CommitsList, - IBranchSelectorStore, - SandboxLayout, - TranslationStore, - TypesCommit -} from '..' -import { Layout } from './layout' - -export const pullRequestFormSchema = z.object({ - title: z.string().min(1, { message: 'Please provide a pull request title' }), - description: z.string().optional() -}) -export type CompareFormFields = z.infer // Automatically generate a type from the schema - -export const DiffModeOptions = [ - { name: 'Split', value: 'Split' }, - { name: 'Unified', value: 'Unified' } -] -export interface SandboxPullRequestCompareProps { - onFormSubmit: (data: CompareFormFields) => void - onFormDraftSubmit: (data: CompareFormFields) => void - onFormCancel: () => void - apiError: string | null - isLoading: boolean - isSuccess: boolean - mergeability?: boolean - onSelectCommit: (commit: CommitSelectorListItem) => void - - selectBranch: (branchTag: BranchSelectorListItem, type: BranchSelectorTab, sourceBranch: boolean) => void - targetBranch: BranchSelectorListItem - sourceBranch: BranchSelectorListItem - diffData: HeaderProps[] - diffStats: TypesDiffStats - isBranchSelected: boolean - setIsBranchSelected: (val: boolean) => void - prBranchCombinationExists: number | null - useTranslationStore: () => TranslationStore - useRepoBranchesStore: () => IBranchSelectorStore - repoId?: string - spaceId?: string - useRepoCommitsStore: () => ICommitSelectorStore - searchCommitQuery: string | null - setSearchCommitQuery: (query: string | null) => void - currentUser?: string - searchQuery: string - setSearchQuery: (query: string) => void -} -/** - * TODO: This code was migrated from V2 and needs to be refactored. - */ -const PullRequestCompare: React.FC = ({ - onFormSubmit, - apiError = null, - isLoading, - isSuccess, - onFormDraftSubmit, - mergeability = false, - selectBranch, - targetBranch, - sourceBranch, - diffData, - diffStats, - setIsBranchSelected, - isBranchSelected, - prBranchCombinationExists, - useTranslationStore, - useRepoBranchesStore, - useRepoCommitsStore, - currentUser, - searchQuery, - setSearchQuery -}) => { - const { commits: commitData } = useRepoCommitsStore() - const formRef = useRef(null) // Create a ref for the form - const [isSubmitted, setIsSubmitted] = useState(false) - const navigate = useNavigate() - const { - register, - handleSubmit, - reset, - - formState: { errors, isValid } - } = useForm({ - resolver: zodResolver(pullRequestFormSchema), - mode: 'onChange', - defaultValues: { - title: '', - description: '' - } - }) - - useEffect(() => { - if (commitData && commitData.length > 0) { - reset({ - title: commitData[commitData.length - 1]?.title, - description: '' - }) - } - }, [commitData, reset]) - - useEffect(() => { - if (isSuccess === true) { - reset() - setIsSubmitted(true) - } - }, [isSuccess]) - - const handleBranchSelection = () => { - setIsBranchSelected(true) // Update state when a branch is selected - } - const [diffMode, setDiffMode] = useState(DiffModeEnum.Split) - const handleDiffModeChange = (value: string) => { - setDiffMode(value === 'Split' ? DiffModeEnum.Split : DiffModeEnum.Unified) - } - return ( - - - - Comparing changes - - - - - Choose two branches to see what's changed or to start a new pull request. If you need to, you can also - compare across forks or learn more about diff comparisons. - - - - { - selectBranch(branchTag, type, false) - handleBranchSelection() - }} - searchQuery={searchQuery} - setSearchQuery={setSearchQuery} - /> - - - { - selectBranch(branchTag, type, true) - handleBranchSelection() - }} - searchQuery={searchQuery} - setSearchQuery={setSearchQuery} - /> - - {isBranchSelected && - !isLoading && ( // Only render this block if isBranchSelected is true - - {mergeability ? ( - <> - - Able to merge. - - These branches can be automatically merged. - - - ) : ( - <> - {apiError === "head branch doesn't contain any new commits." ? ( - <> - - - - Head branch doesn't contain any new commits. - - - ) : ( - <> - - can't be merged. - - You can still create the pull request. - - - )} - - )} - - )} - - - - {!prBranchCombinationExists && ( - -
- - {isBranchSelected ? ( - <> - Discuss and review the changes in this comparison with others. - Learn about pull requests. - - ) : ( - Choose different branches or forks above to discuss and review changes. - )} - -
- - - )} - {prBranchCombinationExists && ( - <> - -
- - <> - PR for this combination of branches already exists. - - -
- {/* */} - - {/* */} - - - )} - - {isBranchSelected ? ( - - - - - - - - - - - - - - {/* TODO: add pagination to this */} - ({ - sha: item.sha, - parent_shas: item.parent_shas, - title: item.title, - message: item.message, - author: item.author, - committer: item.committer - }))} - /> - - - {/* Content for Changes */} - - - - {`Showing ${diffStats.files_changed || 0} changed files with ${diffStats.additions || 0} additions and ${diffStats.deletions || 0} deletions `} - - - - - - - {diffData?.map((item, index) => ( - <> - - - - ))} - - - - - ) : ( - - )} - - - ) -} -interface HeaderProps { - text: string - data?: string - title: string - lang: string - addedLines?: number - removedLines?: number - isBinary?: boolean - deleted?: boolean - unchangedPercentage?: number -} - -const LineTitle: React.FC> = ({ text }) => ( -
-
- {text} - -
-
-
- -
-
-
-) - -const PullRequestAccordion: React.FC<{ - header?: HeaderProps - data?: string - diffMode: DiffModeEnum - currentUser?: string -}> = ({ header, diffMode, currentUser }) => { - const { highlight, wrap, fontsize } = useDiffConfig() - const startingLine = - parseStartingLineIfOne(header?.data ?? '') !== null ? parseStartingLineIfOne(header?.data ?? '') : null - return ( - - - - - - } /> - - -
-
- {startingLine ? ( -
-
{startingLine}
-
- ) : null} - -
-
- - - - - - ) -} - -export { PullRequestCompare } diff --git a/packages/ui/src/views/repo/components/commit-selector/commit-selector.tsx b/packages/ui/src/views/repo/components/commit-selector/commit-selector.tsx index db18c5738..24c3f5c12 100644 --- a/packages/ui/src/views/repo/components/commit-selector/commit-selector.tsx +++ b/packages/ui/src/views/repo/components/commit-selector/commit-selector.tsx @@ -1,9 +1,8 @@ import { FC, useMemo } from 'react' import { Button, DropdownMenu, DropdownMenuTrigger, Icon, Text } from '@/components' -import { TranslationStore } from '@/views' +import { CommitSelectorListItem, TranslationStore } from '@/views' -import { CommitSelectorListItem } from '../../pull-request/compare/components/types' import { CommitSelectorDropdown } from './commit-selector-dropdown' import { ICommitSelectorStore } from './types' diff --git a/packages/ui/src/views/repo/components/commit-selector/types.ts b/packages/ui/src/views/repo/components/commit-selector/types.ts index 363bc8bb5..f6028f321 100644 --- a/packages/ui/src/views/repo/components/commit-selector/types.ts +++ b/packages/ui/src/views/repo/components/commit-selector/types.ts @@ -1,5 +1,4 @@ -import { CommitSelectorListItem } from '@views/repo/pull-request' -import { TypesCommit, TypesListCommitResponse } from '@views/repo/repo-commits' +import { CommitSelectorListItem, TypesCommit, TypesListCommitResponse } from '@/views' export interface ICommitSelectorStore { commits: TypesCommit[] | null diff --git a/packages/ui/src/views/repo/repo-commits/components/commits-list.tsx b/packages/ui/src/views/repo/components/commits-list.tsx similarity index 100% rename from packages/ui/src/views/repo/repo-commits/components/commits-list.tsx rename to packages/ui/src/views/repo/components/commits-list.tsx diff --git a/packages/ui/src/views/repo/pull-request/commits/pull-request-commits.types.ts b/packages/ui/src/views/repo/pull-request/commits/pull-request-commits.types.ts index fa98c715c..e419a2434 100644 --- a/packages/ui/src/views/repo/pull-request/commits/pull-request-commits.types.ts +++ b/packages/ui/src/views/repo/pull-request/commits/pull-request-commits.types.ts @@ -1,4 +1,4 @@ -import { TypesCommit } from '@views/repo/repo-commits' +import { TypesCommit } from '@/views' export interface IPullRequestCommitsStore { // state diff --git a/packages/ui/src/views/repo/pull-request/compare/components/pull-request-compare-button.tsx b/packages/ui/src/views/repo/pull-request/compare/components/pull-request-compare-button.tsx index 1e90e7edb..d5b6aa3a6 100644 --- a/packages/ui/src/views/repo/pull-request/compare/components/pull-request-compare-button.tsx +++ b/packages/ui/src/views/repo/pull-request/compare/components/pull-request-compare-button.tsx @@ -1,4 +1,5 @@ -import { Icon } from '@components/icon' +import { FC, RefObject } from 'react' + import { Button, ButtonGroup, @@ -6,21 +7,23 @@ import { DropdownMenuContent, DropdownMenuGroup, DropdownMenuItem, - DropdownMenuTrigger -} from '@components/index' -import { Text } from '@components/text' -import { CompareFormFields } from '@views/layouts/PullRequestCompareLayout' + DropdownMenuTrigger, + Icon, + Text +} from '@/components' + +import { CompareFormFields } from '../pull-request-compare-page' interface PullRequestCompareButtonProps { isSubmitted: boolean isValid: boolean isLoading: boolean - formRef: React.RefObject + formRef: RefObject onFormSubmit: (data: CompareFormFields) => void onFormDraftSubmit: (data: CompareFormFields) => void } -const PullRequestCompareButton: React.FC = ({ +const PullRequestCompareButton: FC = ({ isSubmitted, isLoading, formRef, @@ -49,6 +52,7 @@ const PullRequestCompareButton: React.FC = ({ onFormSubmit(data as CompareFormFields) // Call the draft submit function } } + return ( <> {!isSubmitted ? ( @@ -57,7 +61,7 @@ const PullRequestCompareButton: React.FC = ({ + +
+
+ +
+
+ +) + +interface PullRequestAccordionProps { + header?: HeaderProps + data?: string + diffMode: DiffModeEnum + currentUser?: string +} + +const PullRequestAccordion: FC = ({ header, diffMode, currentUser }) => { + const { highlight, wrap, fontsize } = useDiffConfig() + const startingLine = + parseStartingLineIfOne(header?.data ?? '') !== null ? parseStartingLineIfOne(header?.data ?? '') : null + return ( + + + + + + } /> + + +
+
+ {startingLine ? ( +
+
{startingLine}
+
+ ) : null} + +
+
+ + + + + + ) +} + +interface PullRequestCompareDiffListProps { + diffStats: TypesDiffStats + diffData: HeaderProps[] + currentUser?: string +} + +const PullRequestCompareDiffList: FC = ({ diffStats, diffData, currentUser }) => { + const [diffMode, setDiffMode] = useState(DiffModeEnum.Split) + const handleDiffModeChange = (value: string) => { + setDiffMode(value === 'Split' ? DiffModeEnum.Split : DiffModeEnum.Unified) + } + + return ( + <> + + +

+ Showing {diffStats.files_changed || 0} changed files + with {diffStats.additions || 0} additions and {diffStats.deletions || 0} deletions +

+ + + + + + + {diffData?.map((item, index) => ( + <> + + + + ))} + + ) +} + +export default PullRequestCompareDiffList diff --git a/packages/ui/src/views/repo/pull-request/compare/components/pull-request-compare-form.tsx b/packages/ui/src/views/repo/pull-request/compare/components/pull-request-compare-form.tsx index 95246fc64..c05dd9de3 100644 --- a/packages/ui/src/views/repo/pull-request/compare/components/pull-request-compare-form.tsx +++ b/packages/ui/src/views/repo/pull-request/compare/components/pull-request-compare-form.tsx @@ -2,7 +2,6 @@ import { forwardRef } from 'react' import { FieldErrors, SubmitHandler, UseFormHandleSubmit, UseFormRegister } from 'react-hook-form' import { Fieldset, Input, Textarea } from '@/components' -import { Text } from '@components/text' import { z } from 'zod' // Define the form schema @@ -31,28 +30,27 @@ const PullRequestCompareForm = forwardRef } return (
-
+
+ Add a title + Add a description