Show diff of deleted files and use chevronIcon left to filepaths

Altinn · Aug 6, 2024 · 509891a · 509891a
1 parent ba80ea2
commit 509891a
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 104 deletions.
diff --git a/backend/src/Designer/Services/Implementation/SourceControlSI.cs b/backend/src/Designer/Services/Implementation/SourceControlSI.cs
@@ -305,7 +305,7 @@ public async Task<Dictionary<string, string>> GetChangedContent(string org, stri
                 var remoteMainCommit = remoteMainBranch.Tip;
 
                 var changes = repo.Diff.Compare<TreeChanges>(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<Patch>(remoteMainCommit.Tree, DiffTargets.WorkingDirectory, new[] { change.Path });
                     fileDiffs[change.Path] = patch.Content;

diff --git a/...areChangesPopover/CommitAndPushContent/FileChangesInfoModal/FileChangesInfoModal.test.tsx b/...areChangesPopover/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/...ts/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FileChangesInfoModal.tsx b/...ts/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 => (
-    <Tag size='small' color={fileStatusToTagColorMapping[fileChange.fileStatus]}>
-      {t(`sync_header.show_changes_modal.file_status_${fileChange.fileStatus}`)}
+  const fileStatusTag: React.ReactElement = (
+    <Tag size='small' color={fileStatusToTagColorMapping[fileStatus]}>
+      {t(`sync_header.show_changes_modal.file_status_${fileStatus}`)}
     </Tag>
   );
-
-  const enableFileDiff =
-    fileChange.fileStatus === 'ModifiedInWorkdir' || fileChange.fileStatus === 'NewInWorkdir';
-
   return (
-    <Table.Row key={fileChange.filePath}>
+    <Table.Row key={filePath}>
       <Table.Cell className={classes.filePath}>
-        <FilePath
-          enableFileDiff={enableFileDiff}
-          filePath={fileChange.filePath}
-          diff={diff}
-          repoDiffStatus={repoDiffStatus}
-        />
+        <FilePath filePath={filePath} diff={diff} repoDiffStatus={repoDiffStatus} />
       </Table.Cell>
-      <Table.Cell className={classes.fileStatus}>{renderFileStatusTag()}</Table.Cell>
+      <Table.Cell className={classes.fileStatus}>{fileStatusTag}</Table.Cell>
     </Table.Row>
   );
 };
diff --git a/...hareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.module.css b/...hareChangesPopover/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/.../ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.test.tsx b/.../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<FilePathProps> = {}) => {
   const defaultProps: FilePathProps = {
     filePath: filePathMock,
-    enableFileDiff: true,
     diff: repoDiffMock,
     repoDiffStatus: 'success',
     ...props,

diff --git a/...nents/ShareChangesPopover/CommitAndPushContent/FileChangesInfoModal/FilePath/FilePath.tsx b/...nents/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,22 +30,23 @@ export const FilePath = ({ enableFileDiff, filePath, diff, repoDiffStatus }: Fil
         className={asButton ? classes.filePathWithDiffContainer : classes.filePathContainer}
         title={filePath}
       >
+        {asButton ? <ChevronRightIcon className={classes.chevronIcon} /> : <div></div>}
         <div className={classes.filePath}>{filePathWithoutName}</div>
         {'/'}
         <strong>{fileName}</strong>
       </div>
     );
   };
 
-  if (!enableFileDiff || repoDiffStatus !== 'success') {
+  if (repoDiffStatus !== 'success') {
     return renderFilePath(false);
   }
   return (
     <details
       className={classes.details}
       title={t('sync_header.show_changes_modal.file_diff_title', { fileName })}
     >
-      <summary className={classes.summaryContent}>{renderFilePath(true)}</summary>
+      <summary>{renderFilePath(true)}</summary>
       <div className={classes.gitDiffViewer}>
         {linesToRender.map((line, index) => {
           return (