From 509891a64bf0ebd3051277a19802c7e777db512f Mon Sep 17 00:00:00 2001 From: andreastanderen Date: Tue, 6 Aug 2024 14:41:25 +0200 Subject: [PATCH] Show diff of deleted files and use chevronIcon left to filepaths --- .../Implementation/SourceControlSI.cs | 2 +- .../FileChangesInfoModal.test.tsx | 81 ++++++------------- .../FileChangesInfoModal.tsx | 24 ++---- .../FilePath/FilePath.module.css | 20 ++--- .../FilePath/FilePath.test.tsx | 12 --- .../FilePath/FilePath.tsx | 11 +-- 6 files changed, 46 insertions(+), 104 deletions(-) diff --git a/backend/src/Designer/Services/Implementation/SourceControlSI.cs b/backend/src/Designer/Services/Implementation/SourceControlSI.cs index 6f7dca58272..d046b7a612c 100644 --- a/backend/src/Designer/Services/Implementation/SourceControlSI.cs +++ b/backend/src/Designer/Services/Implementation/SourceControlSI.cs @@ -305,7 +305,7 @@ public async Task> GetChangedContent(string org, stri var remoteMainCommit = remoteMainBranch.Tip; var changes = repo.Diff.Compare(remoteMainCommit.Tree, DiffTargets.WorkingDirectory); - foreach (var change in changes.Where(change => change.Status is ChangeKind.Modified or ChangeKind.Added)) + foreach (var change in changes) { Patch patch = repo.Diff.Compare(remoteMainCommit.Tree, DiffTargets.WorkingDirectory, new[] { change.Path }); fileDiffs[change.Path] = patch.Content; diff --git a/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FileChangesInfoModal.test.tsx b/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FileChangesInfoModal.test.tsx index 3d306c46d45..1ad09e912d2 100644 --- a/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FileChangesInfoModal.test.tsx +++ b/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FileChangesInfoModal.test.tsx @@ -96,63 +96,30 @@ describe('FileChangesInfoModal', () => { expect(mockGetRepoDiff).toHaveBeenCalledTimes(1); }); - it('should render filePath as clickable when fileStatus is ModifiedInWorkdir or NewInWorkdir', async () => { - const user = userEvent.setup(); - const addedFilePath = `${filePathWithoutNameMock}/addedFile.json`; - await renderFileChangesInfoModalAndWaitForData({ - ...defaultProps, - fileChanges: [ - { - filePath: filePathMock, - fileStatus: fileStatusMock, - }, - { - filePath: addedFilePath, - fileStatus: 'NewInWorkdir', - }, - ], - }); - const modifiedFilePathElement = screen.getByTitle(filePathMock); - const modifiedDiffContentElement = screen.getByRole('group', { - name: textMock('sync_header.show_changes_modal.file_diff_title', { fileName: fileNameMock }), - }); - expect(modifiedDiffContentElement).not.toHaveAttribute('open'); - await user.click(modifiedFilePathElement); - expect(modifiedDiffContentElement).toHaveAttribute('open'); - - const addedFilePathElement = screen.getByTitle(addedFilePath); - const addedDiffContentElement = screen.getByRole('group', { - name: textMock('sync_header.show_changes_modal.file_diff_title', { - fileName: 'addedFile.json', - }), - }); - expect(addedDiffContentElement).not.toHaveAttribute('open'); - await user.click(addedFilePathElement); - expect(addedDiffContentElement).toHaveAttribute('open'); - }); - - it('should not render filePath as clickable when fileStatus is DeleteFromWorkdir', async () => { - const user = userEvent.setup(); - const deletedFilePath = `${filePathWithoutNameMock}/deletedFile.json`; - await renderFileChangesInfoModalAndWaitForData({ - ...defaultProps, - fileChanges: [ - { - filePath: deletedFilePath, - fileStatus: 'DeletedFromWorkdir', - }, - ], - }); - - const deletedFilePathElement = screen.getByTitle(deletedFilePath); - await user.click(deletedFilePathElement); - const deletedDiffContentElement = screen.queryByRole('group', { - name: textMock('sync_header.show_changes_modal.file_diff_title', { - fileName: 'removedFile.json', - }), - }); - expect(deletedDiffContentElement).not.toBeInTheDocument(); - }); + it.each(['ModifiedInWorkdir', 'NewInWorkdir', 'DeletedFromWorkdir'])( + 'should render filePath as clickable when fileStatus is %s', + async (fileStatus) => { + const user = userEvent.setup(); + await renderFileChangesInfoModalAndWaitForData({ + ...defaultProps, + fileChanges: [ + { + filePath: filePathMock, + fileStatus: fileStatus, + }, + ], + }); + const modifiedFilePathElement = screen.getByTitle(filePathMock); + const modifiedDiffContentElement = screen.getByRole('group', { + name: textMock('sync_header.show_changes_modal.file_diff_title', { + fileName: fileNameMock, + }), + }); + expect(modifiedDiffContentElement).not.toHaveAttribute('open'); + await user.click(modifiedFilePathElement); + expect(modifiedDiffContentElement).toHaveAttribute('open'); + }, + ); }); const renderFileChangesInfoModal = (props: FileChangesInfoModalProps = defaultProps) => { diff --git a/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FileChangesInfoModal.tsx b/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FileChangesInfoModal.tsx index 1bdf15bdfaa..a27b32125b7 100644 --- a/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FileChangesInfoModal.tsx +++ b/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FileChangesInfoModal.tsx @@ -107,33 +107,25 @@ export const FileChangesInfoModal = ({ interface FileChangeTableRowProps { fileChange: RepoContentStatus; - diff?: string; // Might be null for deleted files + diff?: string; // Null if diff not fetched successfully repoDiffStatus: 'success' | 'error' | 'pending'; } const FileChangeTableRow = ({ fileChange, diff, repoDiffStatus }: FileChangeTableRowProps) => { + const { filePath, fileStatus } = fileChange; const { t } = useTranslation(); - const fileStatusTag: React.ReactElement => ( - - {t(`sync_header.show_changes_modal.file_status_${fileChange.fileStatus}`)} + const fileStatusTag: React.ReactElement = ( + + {t(`sync_header.show_changes_modal.file_status_${fileStatus}`)} ); - - const enableFileDiff = - fileChange.fileStatus === 'ModifiedInWorkdir' || fileChange.fileStatus === 'NewInWorkdir'; - return ( - + - + - {renderFileStatusTag()} + {fileStatusTag} ); }; diff --git a/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.module.css b/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.module.css index f9c4d4ba57f..bb820168c57 100644 --- a/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.module.css +++ b/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.module.css @@ -1,6 +1,8 @@ .filePathWithDiffContainer { color: var(--fds-semantic-surface-action-default); text-decoration: underline; + align-items: center; + cursor: pointer; } .filePathWithDiffContainer:hover { @@ -10,7 +12,7 @@ .filePathWithDiffContainer, .filePathContainer { display: grid; - grid-template-columns: auto auto 1fr; + grid-template-columns: auto auto auto 1fr; } .filePath { @@ -20,24 +22,16 @@ } .details summary { - display: flex; - align-items: center; - cursor: pointer; -} - -.details summary::-webkit-details-marker { - display: none; + list-style: none; } -.summaryContent::after { - content: '▶'; - font-size: var(--fds-sizing-3); - margin-left: var(--fds-spacing-2); +.chevronIcon { + font-weight: bold; transform: rotate(0deg); transition: transform 0.2s ease-in-out; } -details[open] .summaryContent::after { +details[open] .chevronIcon { transform: rotate(90deg); } diff --git a/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.test.tsx b/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.test.tsx index 6848deb0206..10d7bb6b91f 100644 --- a/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.test.tsx +++ b/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.test.tsx @@ -63,17 +63,6 @@ describe('FilePath', () => { expect(noNewlineElement).not.toBeInTheDocument(); }); - it('should not render diff view if enableFileDiff is false', async () => { - const user = userEvent.setup(); - renderFilePath({ enableFileDiff: false }); - - const filePathElement = screen.getByTitle(filePathMock); - await user.click(filePathElement); - - const diffLineElement = screen.queryByText('+ new line'); - expect(diffLineElement).not.toBeInTheDocument(); - }); - it('should not render filePath as button when repoDiffStatus is error', async () => { const user = userEvent.setup(); renderFilePath({ repoDiffStatus: 'error' }); @@ -121,7 +110,6 @@ describe('FilePath', () => { const renderFilePath = (props: Partial = {}) => { const defaultProps: FilePathProps = { filePath: filePathMock, - enableFileDiff: true, diff: repoDiffMock, repoDiffStatus: 'success', ...props, diff --git a/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.tsx b/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.tsx index fa4391cce4f..fec7a636c18 100644 --- a/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.tsx +++ b/frontend/packages/shared/src/components/GiteaHeader/VersionControlButtons/components/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.tsx @@ -2,22 +2,22 @@ import React from 'react'; import classes from './FilePath.module.css'; import cn from 'classnames'; import { convertPureGitDiffToUserFriendlyDiff } from './FilePathUtils'; +import { ChevronRightIcon } from '@studio/icons'; import { useTranslation } from 'react-i18next'; import { extractFilename, removeFileNameFromPath } from 'app-shared/utils/filenameUtils'; export interface FilePathProps { - enableFileDiff: boolean; filePath: string; diff?: string; // Might be null for deleted files repoDiffStatus: 'success' | 'error' | 'pending'; } -export const FilePath = ({ enableFileDiff, filePath, diff, repoDiffStatus }: FilePathProps) => { +export const FilePath = ({ filePath, diff, repoDiffStatus }: FilePathProps) => { const { t } = useTranslation(); let linesToRender: string[]; - if (enableFileDiff && !!diff) { + if (!!diff) { linesToRender = convertPureGitDiffToUserFriendlyDiff(diff); } @@ -30,6 +30,7 @@ export const FilePath = ({ enableFileDiff, filePath, diff, repoDiffStatus }: Fil className={asButton ? classes.filePathWithDiffContainer : classes.filePathContainer} title={filePath} > + {asButton ? :
}
{filePathWithoutName}
{'/'} {fileName} @@ -37,7 +38,7 @@ export const FilePath = ({ enableFileDiff, filePath, diff, repoDiffStatus }: Fil ); }; - if (!enableFileDiff || repoDiffStatus !== 'success') { + if (repoDiffStatus !== 'success') { return renderFilePath(false); } return ( @@ -45,7 +46,7 @@ export const FilePath = ({ enableFileDiff, filePath, diff, repoDiffStatus }: Fil className={classes.details} title={t('sync_header.show_changes_modal.file_diff_title', { fileName })} > - {renderFilePath(true)} + {renderFilePath(true)}
{linesToRender.map((line, index) => { return (