addressing review comments

iTwin · Nov 2, 2023 · 1b1f965 · 1b1f965
1 parent e00c604
commit 1b1f965
Show file tree

Hide file tree

Showing 13 changed files with 253 additions and 146 deletions.
diff --git a/...es/modules/manage-versions/src/components/InformationPanel/ChangesetInformationPanel.scss b/...es/modules/manage-versions/src/components/InformationPanel/ChangesetInformationPanel.scss
@@ -6,14 +6,12 @@
   width: 400px;
   min-width: 400px;
   .iac-info-panel-body {
-    padding: var(--iui-size-m);
     .iac-info-panel-container {
-      border-top: 1px solid var(--iui-color-background-backdrop);
       padding: var(--iui-size-xs);
-    }
 
-    .iac-info-panel-description {
-      padding: var(--iui-size-xs);
+      & + div {
+        border-top: 1px solid var(--iui-color-border-subtle);
+      }
     }
     .iac-info-panel-details {
       padding: var(--iui-size-xs);
@@ -27,11 +25,6 @@
       }
     }
 
-    .iac-info-panel-connection,
-    .iac-info-panel-description {
-      font-size: var(--iui-font-size-2);
-    }
-
     .iac-info-panel-property {
       display: flex;
       flex: 1;

diff --git a/...odules/manage-versions/src/components/InformationPanel/ChangesetInformationPanel.test.tsx b/...odules/manage-versions/src/components/InformationPanel/ChangesetInformationPanel.test.tsx
@@ -8,6 +8,7 @@ import React from "react";
 import { ConfigProvider } from "../../common/configContext";
 import { MOCKED_CONFIG_PROPS, MockedChangeset } from "../../mocks";
 import { dateTimeFormatOptions, getLocale } from "../../models/utils";
+import { defaultStrings } from "../ManageVersions/ManageVersions";
 import {
   ChangesetInfoPanelProps,
   ChangesetInformationPanel,
@@ -21,8 +22,8 @@ const mockedCreatedDateTime = new Intl.DateTimeFormat(
 const renderComponent = (initialProps?: Partial<ChangesetInfoPanelProps>) => {
   const props: ChangesetInfoPanelProps = {
     changeset: MockedChangeset(),
-    isOpen: true,
     onClose: jest.fn(),
+    stringOverrides: defaultStrings.informationPanelStringOverrides,
     ...initialProps,
   };
   return render(
@@ -46,7 +47,7 @@ describe("ChangesetInformationPanel test", () => {
     ];
 
     const changeset_desc = container.querySelector(
-      ".iac-info-panel-description"
+      ".iac-info-panel-container"
     ) as Element;
 
     const info_panel_details = container.querySelectorAll(
@@ -73,7 +74,7 @@ describe("ChangesetInformationPanel test", () => {
 
   it("should have close icon in the panel header", () => {
     renderComponent();
-    const closeButton = screen.getByTitle("Close");
+    const closeButton = screen.getByLabelText("Close");
     expect(closeButton).toBeTruthy();
   });
 });
diff --git a/...ges/modules/manage-versions/src/components/InformationPanel/ChangesetInformationPanel.tsx b/...ges/modules/manage-versions/src/components/InformationPanel/ChangesetInformationPanel.tsx
@@ -4,99 +4,77 @@
  *--------------------------------------------------------------------------------------------*/
 import "./ChangesetInformationPanel.scss";
 
-import { SvgClose } from "@itwin/itwinui-icons-react";
 import {
-  IconButton,
   InformationPanel,
   InformationPanelBody,
   InformationPanelHeader,
   Text,
 } from "@itwin/itwinui-react";
-import React, { FC } from "react";
+import React from "react";
 
 import { Changeset } from "../../models/changeset";
 import { dateTimeFormatOptions, getLocale } from "../../models/utils";
+import { informationPanelStringOverrides } from "../ManageVersions/types";
 
 export interface ChangesetInfoPanelProps {
   changeset: Changeset;
-  isOpen: boolean;
-  onClose: () => void;
+  onClose: (e: React.MouseEvent<Element, MouseEvent>) => void;
+  stringOverrides: informationPanelStringOverrides;
 }
 
-export const ChangesetInformationPanel: FC<ChangesetInfoPanelProps> = ({
-  isOpen,
-  changeset,
-  onClose,
-}) => {
+export const ChangesetInformationPanel = (props: ChangesetInfoPanelProps) => {
+  const { changeset, onClose, stringOverrides } = props;
   const files: string[] = changeset.synchronizationInfo?.changedFiles
     ? changeset.synchronizationInfo.changedFiles
-    : ["N/A"];
+    : [stringOverrides.noValue];
 
-  const createdDateTime = new Intl.DateTimeFormat(
+  const createdDateTime = new Date(changeset.pushDateTime);
+  const formattedDate = createdDateTime.toLocaleDateString(
     getLocale(),
     dateTimeFormatOptions
-  ).format(new Date(changeset.pushDateTime));
+  );
 
   const renderProperty = (property: string, value: string | undefined) => {
     return (
       <div className="iac-info-panel-details">
-        <span className="iac-info-panel-property">{property}</span>
+        <span className="iac-info-panel-property">{`${property}: `}</span>
         <span className="iac-info-panel-data-value">{value}</span>
       </div>
     );
   };
 
-  const headerActions = () => {
-    return (
-      <IconButton
-        title={"Close"}
-        onClick={() => {
-          onClose();
-        }}
-        styleType="borderless"
-      >
-        <SvgClose />
-      </IconButton>
-    );
-  };
-
-  return isOpen ? (
-    <InformationPanel
-      isOpen={isOpen}
-      className={"iac-info-panel"}
-      resizable={false}
-    >
-      <InformationPanelHeader
-        onClose={undefined}
-        actions={headerActions()}
-        className={"iac-info-panel-header"}
-      >
-        <Text variant="subheading">{"Change  # " + changeset.index}</Text>
+  return (
+    <InformationPanel className={"iac-info-panel"} resizable={false} isOpen>
+      <InformationPanelHeader onClose={onClose}>
+        <Text variant="subheading">
+          {stringOverrides.title + changeset.index}
+        </Text>
       </InformationPanelHeader>
       <InformationPanelBody>
         <div className="iac-info-panel-body">
-          <Text className="iac-info-panel-description">
+          <Text className="iac-info-panel-container">
             {changeset.description}
           </Text>
           <div className="iac-info-panel-container">
-            {renderProperty("Created By: ", changeset.createdBy ?? "")}
-            {renderProperty("Date Created: ", createdDateTime ?? "")}
             {renderProperty(
-              "Application: ",
-              changeset.application.name ?? "N/A"
+              stringOverrides.createdBy,
+              changeset.createdBy ?? ""
+            )}
+            {renderProperty(stringOverrides.createdDate, formattedDate ?? "")}
+            {renderProperty(
+              stringOverrides.application,
+              changeset.application.name ?? stringOverrides.noValue
             )}
           </div>
 
           <div className="iac-info-panel-container">
-            <Text className="iac-info-panel-connection">
-              Connection Attributes
+            <Text variant="leading">
+              {stringOverrides.connectionAttributes}
             </Text>
-            {renderProperty("Changed Files: ", files.join(","))}
+            {renderProperty(stringOverrides.changedFiles, files.join(","))}
           </div>
         </div>
       </InformationPanelBody>
     </InformationPanel>
-  ) : (
-    <></>
   );
 };
diff --git a/packages/modules/manage-versions/src/components/ManageVersions/ChangesTab/ChangesTab.scss b/packages/modules/manage-versions/src/components/ManageVersions/ChangesTab/ChangesTab.scss
@@ -7,4 +7,19 @@
   .iui-table-body .iui-table-cell {
     word-break: break-word;
   }
-}
+
+  .iac-create-version-icon-hidden {
+    visibility: hidden;
+  }
+
+  .iac-changes-tab-actions-btn-group {
+    div {
+      width: 25px;
+    }
+    &:hover {
+      Button {
+        background-color: inherit;
+      }
+    }
+  }
+}
diff --git a/...ages/modules/manage-versions/src/components/ManageVersions/ChangesTab/ChangesTab.test.tsx b/...ages/modules/manage-versions/src/components/ManageVersions/ChangesTab/ChangesTab.test.tsx
@@ -2,7 +2,7 @@
  * Copyright (c) Bentley Systems, Incorporated. All rights reserved.
  * See LICENSE.md in the project root for license terms and full copyright notice.
  *--------------------------------------------------------------------------------------------*/
-import { render, screen, within } from "@testing-library/react";
+import { fireEvent, render, screen, within } from "@testing-library/react";
 import React from "react";
 
 import { ConfigProvider } from "../../../common/configContext";
@@ -22,6 +22,7 @@ const renderComponent = (initialProps?: Partial<ChangesTabProps>) => {
     loadMoreChanges: jest.fn(),
     latestVersion: undefined,
     onVersionCreated: jest.fn(),
+    isLoadingColumn: false,
     ...initialProps,
   };
   return render(
@@ -39,7 +40,7 @@ describe("ChangesTab", () => {
 
     rows.forEach((row, index) => {
       const cells = row.querySelectorAll(".iui-table-cell");
-      expect(cells.length).toBe(7);
+      expect(cells.length).toBe(6);
       expect(cells[0].textContent).toContain(MockedChangeset(index).index);
       expect(cells[1].textContent).toContain(
         MockedChangeset(index).description
@@ -54,7 +55,7 @@ describe("ChangesTab", () => {
       within(cells[5] as HTMLElement).getByTitle(
         defaultStrings.createNamedVersion
       );
-      within(cells[6] as HTMLElement).getByTitle(
+      within(cells[5] as HTMLElement).getByTitle(
         defaultStrings.informationPanel
       );
     });
@@ -81,7 +82,7 @@ describe("ChangesTab", () => {
   });
 
   it("should not show create version icon when changeset already has a version", () => {
-    const { container } = renderComponent({
+    const { container, queryByTitle } = renderComponent({
       changesets: [
         MockedChangeset(1, {
           _links: { namedVersion: { href: "https://test.url" } },
@@ -91,14 +92,15 @@ describe("ChangesTab", () => {
     const rows = container.querySelectorAll(".iui-table-body .iui-table-row");
     expect(rows.length).toBe(1);
 
-    const createVersionicon = screen.queryByTitle(
-      defaultStrings.createNamedVersion
+    const createVersionIcon = queryByTitle(defaultStrings.createNamedVersion);
+    const classAttribute = (createVersionIcon as HTMLElement).getAttribute(
+      "class"
     );
-    expect(createVersionicon).toBeFalsy();
+    expect(classAttribute).toContain("iac-create-version-icon-hidden");
   });
 
   it("should show information panel icon for each changeset row", () => {
-    renderComponent({
+    const { container } = renderComponent({
       changesets: MockedChangesetList(),
     });
     const rowgroup = screen.getAllByRole("rowgroup")[0];
@@ -108,5 +110,9 @@ describe("ChangesTab", () => {
     const rows = within(rowgroup).queryAllByRole("row");
 
     expect(infoIcons.length).toBe(rows.length);
+    //should open information panel
+    fireEvent.click(infoIcons[0]);
+    const panel = container.querySelector(".iac-info-panel");
+    expect(panel).toBeTruthy();
   });
 });