Skip to content

fix: read-only banner, avoid rendering it while isLoading [Backport release/0.5.z] #1139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
carlosthe19916 merged 1 commit into from
Jul 2, 2026
Merged

fix: read-only banner, avoid rendering it while isLoading [Backport release/0.5.z] #1139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 4 additions & 13 deletions client/src/app/components/ReadOnlyButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,11 @@
import React from "react";

import { Button, type ButtonProps, Tooltip } from "@patternfly/react-core";
import { Button, type ButtonProps } from "@patternfly/react-core";

import { READ_ONLY_TOOLTIP, ReadOnlyContext } from "./ReadOnlyContext";
import { ReadOnlyContext } from "./ReadOnlyContext";

/** Button that is automatically aria-disabled with a tooltip when the instance is in read-only mode. */
export const ReadOnlyButton: React.FC<ButtonProps> = (props) => {
const { isReadOnly } = React.useContext(ReadOnlyContext);
const { areMutationsDisabled } = React.useContext(ReadOnlyContext);

if (isReadOnly) {
return (
<Tooltip content={READ_ONLY_TOOLTIP}>
<Button {...props} isAriaDisabled />
</Tooltip>
);
}

return <Button {...props} />;
return <Button {...props} isDisabled={areMutationsDisabled} />;
Comment on lines +8 to +10

@sourcery-ai sourcery-ai Bot Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Switching from isAriaDisabled + tooltip to isDisabled may hurt accessibility and affordance.

With isDisabled, the button is no longer focusable and users lose the tooltip that explained why actions are blocked. This is an accessibility regression for keyboard and assistive tech users, and reduces UX clarity. If removing the tooltip is intentional, consider keeping isAriaDisabled (with an updated label/tooltip) or adding nearby explanatory text when areMutationsDisabled is true.

Suggested implementation:

import { Button, Tooltip, type ButtonProps } from "@patternfly/react-core";
export const ReadOnlyButton: React.FC<ButtonProps> = (props) => {
  const { areMutationsDisabled } = React.useContext(ReadOnlyContext);

  if (areMutationsDisabled) {
    return (
      <Tooltip content={READ_ONLY_TOOLTIP}>
        <Button {...props} isAriaDisabled />
      </Tooltip>
    );
  }

  return <Button {...props} />;
};
  1. Ensure that READ_ONLY_TOOLTIP is defined and imported in this file (for example: import { READ_ONLY_TOOLTIP } from "./constants";), and update its text to describe that mutations/actions are currently disabled.
  2. If your project uses the new JSX runtime and doesn't globally import React, either add import * as React from "react"; at the top of this file or update React.useContext to useContext with a corresponding import { useContext } from "react";.

};
30 changes: 16 additions & 14 deletions client/src/app/components/ReadOnlyContext/ReadOnlyContext.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ const mockedUseFetchTrustifyInfo =
>;

const ReadOnlyConsumer: React.FC = () => {
const { isReadOnly, isLoading } = React.useContext(ReadOnlyContext);
const { isLoading, areMutationsDisabled } = React.useContext(ReadOnlyContext);
return (
<div>
<span data-testid="read-only">{String(isReadOnly)}</span>
<span data-testid="loading">{String(isLoading)}</span>
<span data-testid="mutations-disabled">
{String(areMutationsDisabled)}
</span>
</div>
);
};
Expand All @@ -38,53 +40,53 @@ const renderWithProvider = () => {
};

describe("ReadOnlyContext", () => {
it("provides isReadOnly=true when the endpoint returns readOnly: true", () => {
it("disables mutations when the endpoint returns readOnly: true", () => {
mockedUseFetchTrustifyInfo.mockReturnValue({
trustifyInfo: { version: "0.5.0", readOnly: true },
data: { version: "0.5.0", readOnly: true },
isLoading: false,
error: null,
});

renderWithProvider();

expect(screen.getByTestId("read-only")).toHaveTextContent("true");
expect(screen.getByTestId("loading")).toHaveTextContent("false");
expect(screen.getByTestId("mutations-disabled")).toHaveTextContent("true");
});

it("provides isReadOnly=false when the endpoint returns readOnly: false", () => {
it("allows mutations when the endpoint returns readOnly: false", () => {
mockedUseFetchTrustifyInfo.mockReturnValue({
trustifyInfo: { version: "0.5.0", readOnly: false },
data: { version: "0.5.0", readOnly: false },
isLoading: false,
error: null,
});

renderWithProvider();

expect(screen.getByTestId("read-only")).toHaveTextContent("false");
expect(screen.getByTestId("mutations-disabled")).toHaveTextContent("false");
});

it("treats isReadOnly as true while loading", () => {
it("disables mutations while loading", () => {
mockedUseFetchTrustifyInfo.mockReturnValue({
trustifyInfo: undefined,
data: undefined,
isLoading: true,
error: null,
});

renderWithProvider();

expect(screen.getByTestId("read-only")).toHaveTextContent("true");
expect(screen.getByTestId("loading")).toHaveTextContent("true");
expect(screen.getByTestId("mutations-disabled")).toHaveTextContent("true");
});

it("defaults to isReadOnly=false when the fetch errors", () => {
it("allows mutations when the fetch errors", () => {
mockedUseFetchTrustifyInfo.mockReturnValue({
trustifyInfo: undefined,
data: undefined,
isLoading: false,
error: new Error("network error"),
});

renderWithProvider();

expect(screen.getByTestId("read-only")).toHaveTextContent("false");
expect(screen.getByTestId("mutations-disabled")).toHaveTextContent("false");
});
});
4 changes: 2 additions & 2 deletions client/src/app/components/ReadOnlyContext/ReadOnlyContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import React from "react";

interface IReadOnlyContext {
isReadOnly: boolean;
isLoading: boolean;
areMutationsDisabled: boolean;
}

export const ReadOnlyContext = React.createContext<IReadOnlyContext>({
isReadOnly: false,
isLoading: true,
areMutationsDisabled: true,
});
7 changes: 4 additions & 3 deletions client/src/app/components/ReadOnlyContext/ReadOnlyProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ interface IReadOnlyProvider {
export const ReadOnlyProvider: React.FunctionComponent<IReadOnlyProvider> = ({
children,
}) => {
const { trustifyInfo, isLoading } = useFetchTrustifyInfo();
const isReadOnly = isLoading || (trustifyInfo?.readOnly ?? false);
const { data: trustifyInfo, isLoading } = useFetchTrustifyInfo();
const isReadOnly = trustifyInfo?.readOnly ?? false;
const areMutationsDisabled = isLoading || isReadOnly;

return (
<ReadOnlyContext.Provider value={{ isReadOnly, isLoading }}>
<ReadOnlyContext.Provider value={{ isLoading, areMutationsDisabled }}>
{children}
</ReadOnlyContext.Provider>
);
Expand Down
1 change: 0 additions & 1 deletion client/src/app/components/ReadOnlyContext/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
export * from "./ReadOnlyContext";
export * from "./ReadOnlyProvider";
export * from "./utils";
11 changes: 0 additions & 11 deletions client/src/app/components/ReadOnlyContext/utils.ts
View file
Open in desktop

This file was deleted.

26 changes: 20 additions & 6 deletions client/src/app/layout/default-layout.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,15 @@ vi.mock("@patternfly/react-core", async () => {
};
});

const renderLayout = (isReadOnly: boolean) => {
const renderLayout = ({
isLoading = false,
areMutationsDisabled = false,
}: {
isLoading?: boolean;
areMutationsDisabled?: boolean;
}) => {
return render(
<ReadOnlyContext.Provider value={{ isReadOnly, isLoading: false }}>
<ReadOnlyContext.Provider value={{ isLoading, areMutationsDisabled }}>
<DefaultLayout>
<div data-testid="page-content">Page content</div>
</DefaultLayout>
Expand All @@ -44,19 +50,27 @@ const renderLayout = (isReadOnly: boolean) => {
};

describe("DefaultLayout", () => {
it("shows a read-only banner when isReadOnly is true", () => {
renderLayout(true);
it("shows a read-only banner when mutations are disabled", () => {
renderLayout({ areMutationsDisabled: true });

expect(screen.getByText(/running in read-only mode/i)).toBeInTheDocument();
expect(screen.getByTestId("page-content")).toBeInTheDocument();
});

it("does not show a banner when isReadOnly is false", () => {
renderLayout(false);
it("does not show a banner when mutations are allowed", () => {
renderLayout({ areMutationsDisabled: false });

expect(
screen.queryByText(/running in read-only mode/i),
).not.toBeInTheDocument();
expect(screen.getByTestId("page-content")).toBeInTheDocument();
});

it("does not show a banner while trustify info is loading", () => {
renderLayout({ isLoading: true, areMutationsDisabled: true });

expect(
screen.queryByText(/running in read-only mode/i),
).not.toBeInTheDocument();
});
});
54 changes: 33 additions & 21 deletions client/src/app/layout/default-layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ import { ReadOnlyContext } from "@app/components/ReadOnlyContext";

import { HeaderApp } from "./header";
import { SidebarApp } from "./sidebar";
import { LoadingWrapper } from "@app/components/LoadingWrapper";
import { getAxiosErrorMessage } from "@app/utils/utils";

interface DefaultLayoutProps {
children?: React.ReactNode;
}

export const DefaultLayout: React.FC<DefaultLayoutProps> = ({ children }) => {
const { isReadOnly } = React.useContext(ReadOnlyContext);
const { isLoading, areMutationsDisabled } = React.useContext(ReadOnlyContext);
const pageId = "main-content-page-layout-horizontal-nav";
const PageSkipToContent = (
<SkipToContent href={`#${pageId}`}>Skip to content</SkipToContent>
Expand All @@ -35,27 +37,37 @@ export const DefaultLayout: React.FC<DefaultLayoutProps> = ({ children }) => {
skipToContent={PageSkipToContent}
mainContainerId={pageId}
>
{isReadOnly && (
<Banner
isSticky
status="info"
screenReaderText="Info banner: application is in read-only mode"
>
<Flex
justifyContent={{ default: "justifyContentCenter" }}
alignItems={{ default: "alignItemsCenter" }}
gap={{ default: "gapSm" }}
<LoadingWrapper
isFetching={isLoading}
isFetchingState={<></>}
fetchErrorState={(error) => (
<Banner isSticky status="danger">
{getAxiosErrorMessage(error)}
</Banner>
)}
>
{areMutationsDisabled && (
<Banner
isSticky
status="info"
screenReaderText="Info banner: application is in read-only mode"
>
<FlexItem>
<InfoCircleIcon />
</FlexItem>
<FlexItem>
This instance is running in read-only mode. Uploads, imports, and
other modifications are disabled.
</FlexItem>
</Flex>
</Banner>
)}
<Flex
justifyContent={{ default: "justifyContentCenter" }}
alignItems={{ default: "alignItemsCenter" }}
gap={{ default: "gapSm" }}
>
<FlexItem>
<InfoCircleIcon />
</FlexItem>
<FlexItem>
This instance is running in read-only mode. Uploads, imports,
and other modifications are disabled.
</FlexItem>
</Flex>
</Banner>
)}
</LoadingWrapper>
<PageContentWithDrawerProvider>
{children}
<Notifications />
Expand Down
11 changes: 4 additions & 7 deletions client/src/app/pages/advisory-list/advisory-table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ import type { AdvisorySummary } from "@app/client";
import { ConfirmDialog } from "@app/components/ConfirmDialog.tsx";
import { LabelsAsList } from "@app/components/LabelsAsList.tsx";
import { NotificationsContext } from "@app/components/NotificationsContext";
import {
readOnlyActionProps,
ReadOnlyContext,
} from "@app/components/ReadOnlyContext";
import { ReadOnlyContext } from "@app/components/ReadOnlyContext";
import { SimplePagination } from "@app/components/SimplePagination";
import {
ConditionalTableBody,
Expand All @@ -50,7 +47,7 @@ import { advisoryDeleteDialogProps } from "@app/Constants";

export const AdvisoryTable: React.FC = () => {
const { pushNotification } = React.useContext(NotificationsContext);
const { isReadOnly } = React.useContext(ReadOnlyContext);
const { areMutationsDisabled } = React.useContext(ReadOnlyContext);

const { isFetching, fetchError, tableControls } = React.useContext(
AdvisorySearchContext,
Expand Down Expand Up @@ -224,7 +221,7 @@ export const AdvisoryTable: React.FC = () => {
onClick: () => {
setEditLabelsModalState(item);
},
...readOnlyActionProps(isReadOnly),
isDisabled: areMutationsDisabled,
},
{
title: "Download",
Expand All @@ -241,7 +238,7 @@ export const AdvisoryTable: React.FC = () => {
onClick: () => {
setAdvisoryToDelete(item);
},
...readOnlyActionProps(isReadOnly),
isDisabled: areMutationsDisabled,
},
]}
/>
Expand Down
13 changes: 6 additions & 7 deletions client/src/app/pages/importer-list/importer-list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const getImporterStatus = (importer: Importer): ImporterStatus => {

export const ImporterList: React.FC = () => {
const { pushNotification } = React.useContext(NotificationsContext);
const { isReadOnly } = React.useContext(ReadOnlyContext);
const { areMutationsDisabled } = React.useContext(ReadOnlyContext);

// Actions that each row can trigger
type RowAction = "enable" | "disable" | "run";
Expand Down Expand Up @@ -397,7 +397,7 @@ export const ImporterList: React.FC = () => {
onClick: () => {
prepareActionOnRow("enable", item);
},
...readOnlyActionProps(isReadOnly),
isDisabled: areMutationsDisabled,
},
]
: [
Expand All @@ -406,17 +406,16 @@ export const ImporterList: React.FC = () => {
onClick: () => {
prepareActionOnRow("run", item);
},
isAriaDisabled:
isReadOnly ||
importerStatus === "running",
...readOnlyActionProps(isReadOnly),
isDisabled:
importerStatus === "running" ||
areMutationsDisabled,
},
{
title: "Disable",
onClick: () => {
prepareActionOnRow("disable", item);
},
...readOnlyActionProps(isReadOnly),
isDisabled: areMutationsDisabled,
},
]),
]}
Expand Down
12 changes: 4 additions & 8 deletions client/src/app/pages/sbom-groups/sbom-groups-table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ import type { Group } from "@app/client";
import { ConfirmDialog } from "@app/components/ConfirmDialog.tsx";
import { LoadingWrapper } from "@app/components/LoadingWrapper";
import { NotificationsContext } from "@app/components/NotificationsContext";
import {
readOnlyActionProps,
ReadOnlyContext,
} from "@app/components/ReadOnlyContext";
import { ReadOnlyContext } from "@app/components/ReadOnlyContext";
import { SimplePagination } from "@app/components/SimplePagination";
import { TableCellError } from "@app/components/TableCellError";
import { ConditionalTableBody } from "@app/components/TableControls";
Expand Down Expand Up @@ -155,19 +152,18 @@ const SbomGroupRow: React.FC<{
isExpanded || hasBeenExpanded.current,
);

const { isReadOnly } = React.useContext(ReadOnlyContext);
const { areMutationsDisabled } = React.useContext(ReadOnlyContext);

const actions: IAction[] = [
{
title: "Edit",
onClick: () => onEdit(node),
...readOnlyActionProps(isReadOnly),
isDisabled: areMutationsDisabled,
},
{
title: "Delete",
onClick: () => onDelete(node),
isDisabled: !isReadOnly && !!node.number_of_groups,
...readOnlyActionProps(isReadOnly),
isDisabled: areMutationsDisabled || !!node.number_of_groups,
},
];

Expand Down
Loading
Loading