Skip to content

Fix single perspective icon and text in NavHeader #16663

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

Open
vojtechszocs wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix single perspective icon and text in NavHeader #16663

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
1 change: 1 addition & 0 deletions frontend/@types/console/window.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ declare interface Window {
GOOS: string;
graphqlBaseURL: string;
developerCatalogCategories: string;
/** JSON encoded configuration for the console's perspectives override */
perspectives: string;
developerCatalogTypes: string;
userSettingsLocation: string;
Expand Down
11 changes: 5 additions & 6 deletions frontend/packages/console-app/src/actions/providers/perspective-state-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import type { SetFeatureFlag } from '@console/dynamic-plugin-sdk';
import type { Perspective } from '@console/shared/src/hooks/usePerspectives';
import { hasReviewAccess } from '@console/shared/src/hooks/usePerspectives';
import {
hasReviewAccess,
PerspectiveVisibilityState,
} from '@console/shared/src/hooks/usePerspectives';
overridePerspectives,
} from '@console/shared/src/utils/override-perspectives';
import { FLAG_DEVELOPER_PERSPECTIVE } from '../../consts';

export const useDeveloperPerspectiveStateProvider = (setFeatureFlag: SetFeatureFlag) => {
if (!window.SERVER_FLAGS.perspectives) {
if (!overridePerspectives) {
setFeatureFlag(FLAG_DEVELOPER_PERSPECTIVE, true);
} else {
const perspectives: Perspective[] = JSON.parse(window.SERVER_FLAGS.perspectives);
const devPerspective = perspectives?.find((p) => p.id === 'dev');
const devPerspective = overridePerspectives.find((p) => p.id === 'dev');
if (!devPerspective) {
setFeatureFlag(FLAG_DEVELOPER_PERSPECTIVE, true);
} else if (devPerspective.visibility.state === PerspectiveVisibilityState.Disabled) {
Expand Down
31 changes: 6 additions & 25 deletions frontend/packages/console-app/src/components/detect-context/PerspectiveConfiguration.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ import {
} from '@patternfly/react-core';
import { safeDump } from 'js-yaml';
import { useTranslation } from 'react-i18next';
import type {
Perspective as PerspectiveExtension,
AccessReviewResourceAttributes,
} from '@console/dynamic-plugin-sdk/src';
import type { Perspective as PerspectiveExtension } from '@console/dynamic-plugin-sdk/src';
import { isPerspective } from '@console/dynamic-plugin-sdk/src';
import type { K8sResourceKind } from '@console/internal/module/k8s';
import { useExtensions } from '@console/plugin-sdk/src/api/useExtensions';
Expand All @@ -29,27 +26,11 @@ import { SaveStatus } from '@console/shared/src/components/cluster-configuration
import { useConsoleOperatorConfig } from '@console/shared/src/components/cluster-configuration/useConsoleOperatorConfig';
import { useDebounceCallback } from '@console/shared/src/hooks/useDebounceCallback';
import { useTelemetry } from '@console/shared/src/hooks/useTelemetry';

enum PerspectiveVisibilityState {
Enabled = 'Enabled',
Disabled = 'Disabled',
AccessReview = 'AccessReview',
}

type PerspectiveAccessReview = {
required?: AccessReviewResourceAttributes[];
missing?: AccessReviewResourceAttributes[];
};

type PerspectiveVisibility = {
state: PerspectiveVisibilityState;
accessReview?: PerspectiveAccessReview;
};

type Perspective = {
id: string;
visibility: PerspectiveVisibility;
};
import type {
PerspectiveVisibility,
Perspective,
} from '@console/shared/src/utils/override-perspectives';
import { PerspectiveVisibilityState } from '@console/shared/src/utils/override-perspectives';

type PerspectivesConsoleConfig = K8sResourceKind & {
spec: {
Expand Down
20 changes: 13 additions & 7 deletions ...packages/console-app/src/components/detect-context/__tests__/PerspectiveDetector.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,20 @@ import { render, waitFor } from '@testing-library/react';
import { useLocation } from 'react-router';
import type { Perspective } from '@console/dynamic-plugin-sdk';
import type { LoadedExtension } from '@console/dynamic-plugin-sdk/src/types';
import {
usePerspectives,
PerspectiveVisibilityState,
} from '@console/shared/src/hooks/usePerspectives';
import type { Perspective as PerspectiveType } from '@console/shared/src/hooks/usePerspectives';
import { usePerspectives } from '@console/shared/src/hooks/usePerspectives';
import type { Perspective as PerspectiveType } from '@console/shared/src/utils/override-perspectives';
import { PerspectiveVisibilityState } from '@console/shared/src/utils/override-perspectives';
import PerspectiveDetector from '../PerspectiveDetector';

let mockOverridePerspectives: PerspectiveType[] | undefined;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

jest.mock('@console/shared/src/utils/override-perspectives', () => ({
...jest.requireActual('@console/shared/src/utils/override-perspectives'),
get overridePerspectives() {
return mockOverridePerspectives;
},
}));

jest.mock('@console/shared/src/hooks/usePerspectives', () => ({
...jest.requireActual('@console/shared/src/hooks/usePerspectives'),
usePerspectives: jest.fn(),
Expand Down Expand Up @@ -94,7 +101,7 @@ describe('PerspectiveDetector', () => {
});

it('should set admin as default perspective when all perspectives are disabled', async () => {
const perspectives: PerspectiveType[] = [
mockOverridePerspectives = [
{
id: 'dev',
visibility: {
Expand Down Expand Up @@ -122,7 +129,6 @@ describe('PerspectiveDetector', () => {
},
},
];
window.SERVER_FLAGS.perspectives = JSON.stringify(perspectives);

let promiseResolver: (value: () => [boolean, boolean]) => void;
const testPromise = new Promise<() => [boolean, boolean]>(
Expand Down
43 changes: 21 additions & 22 deletions frontend/packages/console-app/src/components/nav/NavHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,23 @@ const NavHeader: FC<NavHeaderProps> = ({ onPerspectiveSelected }) => {
/>
));

const { icon, name } = useMemo(
const { icon, name } = useMemo<{
icon: Perspective['properties']['icon'];
name: Perspective['properties']['name'];
}>(
() =>
perspectiveExtensions.find((p) => p?.properties?.id === activePerspective)?.properties ??
perspectiveExtensions[0]?.properties ?? { icon: null, name: null },
[activePerspective, perspectiveExtensions],
perspectiveExtensions[0]?.properties ?? { icon: null, name: t('Core platform') },
[activePerspective, perspectiveExtensions, t],
);

const ActivePerspectiveIcon = icon ? (
<AsyncComponent
loader={() => icon().then((m) => m.default)}
LoadingComponent={IconLoadingComponent}
/>
) : (
<RhUiGearGroupFillIcon />
);

return perspectiveDropdownItems.length > 1 ? (
Expand All @@ -99,20 +111,11 @@ const NavHeader: FC<NavHeaderProps> = ({ onPerspectiveSelected }) => {
isExpanded={isPerspectiveDropdownOpen}
ref={toggleRef}
onClick={() => togglePerspectiveOpen()}
icon={
icon && (
<AsyncComponent
loader={() => icon().then((m) => m.default)}
LoadingComponent={IconLoadingComponent}
/>
)
}
icon={ActivePerspectiveIcon}
>
{name && (
<Title headingLevel="h2" size="md">
{name}
</Title>
)}
<Title headingLevel="h2" size="md">
{name}
</Title>
</MenuToggle>
)}
popperProps={{
Expand All @@ -123,13 +126,9 @@ const NavHeader: FC<NavHeaderProps> = ({ onPerspectiveSelected }) => {
</Select>
</div>
) : (
<div
data-test="perspective-switcher-toggle"
data-test-id="perspective-switcher-toggle"
id="core-platform-perspective"
>
<div data-test="perspective-switcher-toggle" data-test-id="perspective-switcher-toggle">
<Title headingLevel="h2" size="md">
<RhUiGearGroupFillIcon /> {t('Core platform')}
{ActivePerspectiveIcon} {name}
</Title>
</div>
);
Expand Down
31 changes: 21 additions & 10 deletions frontend/packages/console-app/src/components/nav/__tests__/NavHeader.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
import { screen, waitForElementToBeRemoved } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { renderWithProviders } from '@console/shared/src/test-utils/unit-test-utils';
import type { Perspective } from '@console/shared/src/utils/override-perspectives';
import { PerspectiveVisibilityState } from '@console/shared/src/utils/override-perspectives';
import NavHeader from '../NavHeader';
import { renderWithPerspective } from './navTestUtils';

let mockOverridePerspectives: Perspective[];

@coderabbitai coderabbitai Bot Jun 23, 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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the type of mockOverridePerspectives to allow undefined.

The variable is typed as Perspective[] but should be Perspective[] | undefined to match the actual return type of overridePerspectives. The afterEach blocks at lines 110 and 130 explicitly reset this variable to undefined.

🔧 Proposed fix 
-let mockOverridePerspectives: Perspective[];
+let mockOverridePerspectives: Perspective[] | undefined;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let mockOverridePerspectives: Perspective[];
let mockOverridePerspectives: Perspective[] | undefined;
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@frontend/packages/console-app/src/components/nav/__tests__/NavHeader.spec.tsx`
at line 9, The variable mockOverridePerspectives is declared with type
Perspective[] but is reset to undefined in the afterEach cleanup blocks, so the
type declaration does not match the actual possible values. Update the type
annotation of mockOverridePerspectives from Perspective[] to Perspective[] |
undefined to accurately reflect that it can be either an array of Perspective
objects or undefined.


jest.mock('@console/shared/src/utils/override-perspectives', () => ({
...jest.requireActual('@console/shared/src/utils/override-perspectives'),
get overridePerspectives() {
return mockOverridePerspectives;
},
}));

jest.mock('@console/internal/components/utils/async', () => ({
AsyncComponent: () => null,
}));
Expand Down Expand Up @@ -89,14 +100,14 @@ describe('NavHeader', () => {

describe('when only one perspective is available', () => {
beforeEach(() => {
window.SERVER_FLAGS.perspectives = JSON.stringify([
{ id: 'admin', visibility: { state: 'Enabled' } },
{ id: 'dev', visibility: { state: 'Disabled' } },
]);
mockOverridePerspectives = [
{ id: 'admin', visibility: { state: PerspectiveVisibilityState.Enabled } },
{ id: 'dev', visibility: { state: PerspectiveVisibilityState.Disabled } },
];
});

afterEach(() => {
delete window.SERVER_FLAGS.perspectives;
mockOverridePerspectives = undefined;
});

it('should render static label instead of dropdown', () => {
Expand All @@ -109,14 +120,14 @@ describe('NavHeader', () => {

describe('when all perspectives are disabled', () => {
beforeEach(() => {
window.SERVER_FLAGS.perspectives = JSON.stringify([
{ id: 'admin', visibility: { state: 'Disabled' } },
{ id: 'dev', visibility: { state: 'Disabled' } },
]);
mockOverridePerspectives = [
{ id: 'admin', visibility: { state: PerspectiveVisibilityState.Disabled } },
{ id: 'dev', visibility: { state: PerspectiveVisibilityState.Disabled } },
];
});

afterEach(() => {
delete window.SERVER_FLAGS.perspectives;
mockOverridePerspectives = undefined;
});

it('should fall back to static label for admin perspective', () => {
Expand Down
Loading