From 82219af14e87940820ff9d80251ecad0b2f86384 Mon Sep 17 00:00:00 2001 From: Sakshi Gupta Date: Wed, 7 Jan 2026 14:43:45 +0530 Subject: [PATCH] fixed review comments --- src/Common/Pager/index.tsx | 5 +-- .../CommandBar/CopyJobCommandBar.test.tsx | 8 ++-- .../CommandBar/CopyJobCommandBar.tsx | 2 +- .../ContainerCopy/CommandBar/Utils.test.ts | 38 +++++++++---------- .../ContainerCopy/CommandBar/Utils.ts | 7 ++-- .../Components/CopyJobStatusWithIcon.tsx | 2 +- src/less/ThemeSystem.less | 2 + 7 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/Common/Pager/index.tsx b/src/Common/Pager/index.tsx index ec6091b46..f85209c24 100644 --- a/src/Common/Pager/index.tsx +++ b/src/Common/Pager/index.tsx @@ -31,7 +31,6 @@ const iconButtonStyles = { outline: "none", }, }; -const textStyle: React.CSSProperties = { color: "var(--colorNeutralForeground1)" }; const Pager: React.FC = ({ startIndex, @@ -60,7 +59,7 @@ const Pager: React.FC = ({ return (
{showItemCount && ( - + Showing {startIndex + 1} - {endIndex} of {totalCount} items )} @@ -83,7 +82,7 @@ const Pager: React.FC = ({ disabled={disabled || currentPage === 1} styles={iconButtonStyles} /> - + Page {currentPage} of {totalPages} { render(); - expect(mockGetCommandBarButtons).toHaveBeenCalledWith(mockExplorer); + expect(mockGetCommandBarButtons).toHaveBeenCalledWith(mockExplorer, false); expect(mockGetCommandBarButtons).toHaveBeenCalledTimes(1); }); @@ -163,7 +163,7 @@ describe("CopyJobCommandBar", () => { render(); - expect(mockGetCommandBarButtons).toHaveBeenCalledWith(mockExplorer); + expect(mockGetCommandBarButtons).toHaveBeenCalledWith(mockExplorer, false); expect(mockConvertButton.mock.calls[0][0]).toEqual(mockCommandButtonProps); }); @@ -175,11 +175,11 @@ describe("CopyJobCommandBar", () => { mockConvertButton.mockReturnValue([]); const { rerender } = render(); - expect(mockGetCommandBarButtons).toHaveBeenCalledWith(mockExplorer1); + expect(mockGetCommandBarButtons).toHaveBeenCalledWith(mockExplorer1, false); rerender(); - expect(mockGetCommandBarButtons).toHaveBeenCalledWith(mockExplorer2); + expect(mockGetCommandBarButtons).toHaveBeenCalledWith(mockExplorer2, false); expect(mockGetCommandBarButtons).toHaveBeenCalledTimes(2); }); }); diff --git a/src/Explorer/ContainerCopy/CommandBar/CopyJobCommandBar.tsx b/src/Explorer/ContainerCopy/CommandBar/CopyJobCommandBar.tsx index 9cd625861..9cd2f5002 100644 --- a/src/Explorer/ContainerCopy/CommandBar/CopyJobCommandBar.tsx +++ b/src/Explorer/ContainerCopy/CommandBar/CopyJobCommandBar.tsx @@ -18,7 +18,7 @@ const CopyJobCommandBar: React.FC = ({ explorer }) => { }, }; - const commandBarItems: CommandButtonComponentProps[] = getCommandBarButtons(explorer); + const commandBarItems: CommandButtonComponentProps[] = getCommandBarButtons(explorer, isDarkMode); const controlButtons: ICommandBarItemProps[] = CommandBarUtil.convertButton(commandBarItems, backgroundColor); return ( diff --git a/src/Explorer/ContainerCopy/CommandBar/Utils.test.ts b/src/Explorer/ContainerCopy/CommandBar/Utils.test.ts index 6713869cc..4e049ee68 100644 --- a/src/Explorer/ContainerCopy/CommandBar/Utils.test.ts +++ b/src/Explorer/ContainerCopy/CommandBar/Utils.test.ts @@ -50,7 +50,7 @@ describe("CommandBar Utils", () => { describe("getCommandBarButtons", () => { it("should return an array of command button props", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); expect(buttons).toBeDefined(); expect(Array.isArray(buttons)).toBe(true); @@ -58,7 +58,7 @@ describe("CommandBar Utils", () => { }); it("should include create copy job button", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); const createButton = buttons[0]; expect(createButton).toBeDefined(); @@ -70,7 +70,7 @@ describe("CommandBar Utils", () => { }); it("should include refresh button", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); const refreshButton = buttons[1]; expect(refreshButton).toBeDefined(); @@ -80,7 +80,7 @@ describe("CommandBar Utils", () => { }); it("should include feedback button when platform is Portal", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); expect(buttons.length).toBe(4); @@ -111,7 +111,7 @@ describe("CommandBar Utils", () => { }); it("should call openCreateCopyJobPanel when create button is clicked", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); const createButton = buttons[0]; createButton.onCommandClick({} as React.SyntheticEvent); @@ -121,7 +121,7 @@ describe("CommandBar Utils", () => { }); it("should call refreshJobList when refresh button is clicked", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); const refreshButton = buttons[1]; refreshButton.onCommandClick({} as React.SyntheticEvent); @@ -130,7 +130,7 @@ describe("CommandBar Utils", () => { }); it("should call openContainerCopyFeedbackBlade when feedback button is clicked", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); const feedbackButton = buttons[3]; feedbackButton.onCommandClick({} as React.SyntheticEvent); @@ -139,7 +139,7 @@ describe("CommandBar Utils", () => { }); it("should return buttons with correct icon sources", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); expect(buttons[0].iconSrc).toBeDefined(); expect(buttons[0].iconAlt).toBe("Create Copy Job"); @@ -160,14 +160,14 @@ describe("CommandBar Utils", () => { return selector(state); }); - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); const refreshButton = buttons[1]; expect(() => refreshButton.onCommandClick({} as React.SyntheticEvent)).not.toThrow(); }); it("should set hasPopup to false for all buttons", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); buttons.forEach((button) => { expect(button.hasPopup).toBe(false); @@ -175,7 +175,7 @@ describe("CommandBar Utils", () => { }); it("should set commandButtonLabel to undefined for all buttons", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); buttons.forEach((button) => { expect(button.commandButtonLabel).toBeUndefined(); @@ -183,7 +183,7 @@ describe("CommandBar Utils", () => { }); it("should respect disabled state when provided", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); buttons.forEach((button) => { expect(button.disabled).toBe(false); @@ -191,7 +191,7 @@ describe("CommandBar Utils", () => { }); it("should return CommandButtonComponentProps with all required properties", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); buttons.forEach((button: CommandButtonComponentProps) => { expect(button).toHaveProperty("iconSrc"); @@ -206,7 +206,7 @@ describe("CommandBar Utils", () => { }); it("should maintain button order: create, refresh, themeToggle, feedback", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); expect(buttons[0].tooltipText).toBe("Create Copy Job"); expect(buttons[1].tooltipText).toBe("Refresh"); @@ -217,7 +217,7 @@ describe("CommandBar Utils", () => { describe("Button click handlers", () => { it("should execute click handlers without errors", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); buttons.forEach((button) => { expect(() => button.onCommandClick({} as React.SyntheticEvent)).not.toThrow(); @@ -225,7 +225,7 @@ describe("CommandBar Utils", () => { }); it("should call correct action for each button", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); buttons[0].onCommandClick({} as React.SyntheticEvent); expect(Actions.openCreateCopyJobPanel).toHaveBeenCalledWith(mockExplorer); @@ -240,7 +240,7 @@ describe("CommandBar Utils", () => { describe("Accessibility", () => { it("should have aria labels for all buttons", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); buttons.forEach((button) => { expect(button.ariaLabel).toBeDefined(); @@ -250,7 +250,7 @@ describe("CommandBar Utils", () => { }); it("should have tooltip text for all buttons", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); buttons.forEach((button) => { expect(button.tooltipText).toBeDefined(); @@ -260,7 +260,7 @@ describe("CommandBar Utils", () => { }); it("should have icon alt text for all buttons", () => { - const buttons = getCommandBarButtons(mockExplorer); + const buttons = getCommandBarButtons(mockExplorer, false); buttons.forEach((button) => { expect(button.iconAlt).toBeDefined(); diff --git a/src/Explorer/ContainerCopy/CommandBar/Utils.ts b/src/Explorer/ContainerCopy/CommandBar/Utils.ts index e79dcc052..d00b3788d 100644 --- a/src/Explorer/ContainerCopy/CommandBar/Utils.ts +++ b/src/Explorer/ContainerCopy/CommandBar/Utils.ts @@ -12,9 +12,8 @@ import ContainerCopyMessages from "../ContainerCopyMessages"; import { MonitorCopyJobsRefState } from "../MonitorCopyJobs/MonitorCopyJobRefState"; import { CopyJobCommandBarBtnType } from "../Types/CopyJobTypes"; -function getCopyJobBtns(explorer: Explorer): CopyJobCommandBarBtnType[] { +function getCopyJobBtns(explorer: Explorer, isDarkMode: boolean): CopyJobCommandBarBtnType[] { const monitorCopyJobsRef = MonitorCopyJobsRefState((state) => state.ref); - const isDarkMode = useThemeStore.getState().isDarkMode; const buttons: CopyJobCommandBarBtnType[] = [ { key: "createCopyJob", @@ -66,6 +65,6 @@ function btnMapper(config: CopyJobCommandBarBtnType): CommandButtonComponentProp }; } -export function getCommandBarButtons(explorer: Explorer): CommandButtonComponentProps[] { - return getCopyJobBtns(explorer).map(btnMapper); +export function getCommandBarButtons(explorer: Explorer, isDarkMode: boolean): CommandButtonComponentProps[] { + return getCopyJobBtns(explorer, isDarkMode).map(btnMapper); } diff --git a/src/Explorer/ContainerCopy/MonitorCopyJobs/Components/CopyJobStatusWithIcon.tsx b/src/Explorer/ContainerCopy/MonitorCopyJobs/Components/CopyJobStatusWithIcon.tsx index 122e005c8..1fb996639 100644 --- a/src/Explorer/ContainerCopy/MonitorCopyJobs/Components/CopyJobStatusWithIcon.tsx +++ b/src/Explorer/ContainerCopy/MonitorCopyJobs/Components/CopyJobStatusWithIcon.tsx @@ -23,7 +23,7 @@ const iconMap: Partial> = { const statusIconColors: Partial> = { [CopyJobStatusType.Failed]: "var(--colorPaletteRedForeground1)", [CopyJobStatusType.Faulted]: "var(--colorPaletteRedForeground1)", - [CopyJobStatusType.Completed]: "#107c10", // Green color for success + [CopyJobStatusType.Completed]: "var(--colorSuccessGreen)", [CopyJobStatusType.InProgress]: "var(--colorBrandForeground1)", [CopyJobStatusType.Running]: "var(--colorBrandForeground1)", [CopyJobStatusType.Partitioning]: "var(--colorBrandForeground1)", diff --git a/src/less/ThemeSystem.less b/src/less/ThemeSystem.less index 6fdeed790..05298d2e6 100644 --- a/src/less/ThemeSystem.less +++ b/src/less/ThemeSystem.less @@ -12,6 +12,7 @@ --colorCompoundBrandStroke1: @SelectionColor; --colorBrandForeground1: @LinkColor; --colorPaletteRedForeground1: @ErrorColor; + --colorSuccessGreen: #107c10; --overlayBackground: rgba(0, 0, 0, 0.4); --colorBrandBackground: @SelectionColor; --colorBrandBackgroundHover: @AccentMediumHigh; @@ -32,6 +33,7 @@ body.isDarkMode { --colorCompoundBrandStroke1: #4db6e8; --colorBrandForeground1: #4db6e8; --colorPaletteRedForeground1: #f25d5d; + --colorSuccessGreen: #107c10; --overlayBackground: rgba(0, 0, 0, 0.4); --colorBrandBackground: #0078d4; --colorBrandBackgroundHover: #106ebe;