Skip to content

Implement Topology's List/Graph switch toggle (#10018)#251

Open
keithchong wants to merge 1 commit into
redhat-developer:mainfrom
keithchong:10018-GraphListToggle
Open

Implement Topology's List/Graph switch toggle (#10018)#251
keithchong wants to merge 1 commit into
redhat-developer:mainfrom
keithchong:10018-GraphListToggle

Conversation

@keithchong

Copy link
Copy Markdown
Collaborator

See GITOPS-10018 for details

Signed-off-by: Keith Chong <kykchong@redhat.com>
@openshift-ci openshift-ci Bot requested a review from wtam2018 June 29, 2026 12:26
@keithchong keithchong requested a review from aali309 June 29, 2026 12:26
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added saved list/graph view switching for application resources and ApplicationSet applications.
    • Introduced topology view labels for List view and Graph view in additional languages.
  • Bug Fixes

    • Improved resource and filter help text clarity across locales.
    • Fixed spacing and wording in several UI messages for a cleaner reading experience.
    • Updated the healthy status icon styling for more consistent display.
  • Documentation

    • Expanded and corrected translated UI strings in English, Japanese, Korean, and Chinese.

Walkthrough

Introduces a shared GitOpsViewType enum and a GitOpsViewSwitcher toggle button that switches between graph and list views with persisted user settings. New ApplicationResourcesView and ApplicationSetApplicationsView components encapsulate the view-switching logic; ApplicationResourcesTab and ApplicationList are refactored to delegate to these new components. Locale files are updated with topology view labels and minor string fixes.

Changes

Graph/List View Switcher Feature

Layer / File(s) Summary
GitOpsViewType enum and setting keys
src/gitops/components/shared/GitOpsViewType.ts, src/gitops/components/application/ApplicationResourcesViewType.ts
Defines GitOpsViewType enum (graph/list) and exports persisted setting-key constants; re-exports them as ApplicationResourcesViewType.
GitOpsViewSwitcher button and layout styles
src/gitops/components/shared/GitOpsViewSwitcher.tsx, src/gitops/components/application/ApplicationResourcesToolbar.tsx, src/gitops/components/shared/GitOpsGraphListView.scss
Implements a tooltip-wrapped link button toggling view type, a thin toolbar wrapper with fixed testId, and SCSS flex-column layout for the graph/list container.
ApplicationResourcesView: sort, filter, columns, rows, graph/list rendering
src/gitops/components/application/ApplicationResourcesView.tsx
New component that sorts and filters ApplicationResourceStatus[], exports useResourceColumnsDV/useResourceRowsDV, builds ResourceActionsCell with useResourceActionsProvider, and renders either GitOpsDataViewTable or ApplicationGraphView.
ApplicationResourcesTab refactored
src/gitops/components/application/ApplicationResourcesTab.tsx, src/gitops/components/application/graph/nodes/ApplicationNode.tsx
Tab now persists view type via useUserSettings, derives argoBaseURL, and delegates all table/graph rendering to ApplicationResourcesView; HeartIcon healthy color switches from fill to color.
ApplicationSetApplicationsView
src/gitops/components/shared/ApplicationSetApplicationsView.tsx
New component that persists appset view type, renders toolbar with optional ListPageFilter and GitOpsViewSwitcher, and switches between GitOpsDataViewTable and ApplicationSetGraphView.
ApplicationList refactored
src/gitops/components/shared/ApplicationList.tsx
Replaces inline appset graph container with ApplicationSetApplicationsView; non-appset path continues to render GitOpsDataViewTable directly.
Locale updates
locales/*/topology.json, locales/*/plugin__gitops-plugin.json
Adds topology.json files (en/ja/ko/zh) with "List view"/"Graph view" labels; reorders and reformats plugin strings for application resources, ImageUpdater, and ApplicationSet filter text across all four locales.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description only points to the issue and does not describe the changeset itself. Add a brief summary of the main changes or affected components instead of only referencing the issue.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly names the Topology list/graph toggle work and matches the main change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 233 lines in your changes missing coverage. Please review.
✅ Project coverage is 11.62%. Comparing base (01e9db1) to head (d422cc6).

Files with missing lines Patch % Lines
...omponents/application/ApplicationResourcesView.tsx 0.00% 132 Missing and 6 partials ⚠️
...mponents/shared/ApplicationSetApplicationsView.tsx 0.00% 33 Missing and 2 partials ⚠️
...components/application/ApplicationResourcesTab.tsx 0.00% 17 Missing and 2 partials ⚠️
...rc/gitops/components/shared/GitOpsViewSwitcher.tsx 0.00% 18 Missing ⚠️
src/gitops/components/shared/ApplicationList.tsx 0.00% 9 Missing ⚠️
src/gitops/components/shared/GitOpsViewType.ts 0.00% 5 Missing and 1 partial ⚠️
...onents/application/ApplicationResourcesToolbar.tsx 0.00% 4 Missing ⚠️
...onents/application/ApplicationResourcesViewType.ts 0.00% 3 Missing ⚠️
...onents/application/graph/nodes/ApplicationNode.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #251      +/-   ##
==========================================
- Coverage   11.92%   11.62%   -0.31%     
==========================================
  Files         154      160       +6     
  Lines        6272     6445     +173     
  Branches     2028     2210     +182     
==========================================
+ Hits          748      749       +1     
+ Misses       5524     5464      -60     
- Partials        0      232     +232     
Flag Coverage Δ
unit-tests 11.62% <0.00%> (-0.31%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🤖 Prompt for all review comments with 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.

Inline comments:
In `@src/gitops/components/application/ApplicationResourcesTab.tsx`:
- Around line 69-76: The Argo CD URL is being constructed before the server
settings are available, so `ArgoCDLink` and the resource actions can receive a
malformed `argoBaseURL` like `:///applications/...`. Update
`ApplicationResourcesTab` so the URL is only built and passed once `argoServer`
has valid protocol/host values, and guard the render of the link/actions until
the full URL is available; apply the same check wherever `argoBaseURL` is used
in the referenced block.
- Line 8: The import block in ApplicationResourcesTab should be reformatted to
match Prettier’s expected style so ESLint stops flagging it. Update the import
statement near the top of ApplicationResourcesTab.tsx using the existing React
Core import as the target, and run the file through Prettier so the formatting
is automatically corrected.

In `@src/gitops/components/application/ApplicationResourcesView.tsx`:
- Around line 161-164: The sync wave handling in ApplicationResourcesView is
treating valid value 0 as missing because it uses a fallback that coerces falsy
values, so wave 0 gets sorted/displayed like absent data. Update the sync-wave
branches in the comparator/rendering logic to use a nullish check instead of a
falsy fallback so 0 is preserved, and apply the same fix anywhere else the
syncWave value is normalized in this component.
- Line 18: The import block in ApplicationResourcesView has a Prettier
formatting issue that is causing ESLint to fail. Reformat the
`@patternfly/react-core` import so it matches the project’s Prettier style,
keeping the same imported symbols (EmptyState, EmptyStateBody, Flex, FlexItem,
Stack, StackItem) but adjusting the layout/spacing to the formatter’s expected
output.

In `@src/gitops/components/shared/ApplicationList.tsx`:
- Line 256: The graph view is still receiving the unsearched filteredData set,
so URL search filters can be ignored in appset graph mode. Update the
ApplicationList render path to pass the same search-filtered collection used by
rows and isEmpty, namely filteredBySearch, into the graph view via
filteredApplications. Keep the fix localized to the ApplicationList component so
the list and graph stay in sync.
- Line 114: The destructuring assignment in
ApplicationList/useGitOpsDataViewSort needs Prettier formatting to satisfy lint.
Reformat the declaration so the imported values from
useGitOpsDataViewSort(columnSortConfig) follow the project’s standard line
wrapping and spacing, keeping the same symbols searchParams, sortBy, direction,
and getSortParams intact.

In `@src/gitops/components/shared/ApplicationSetApplicationsView.tsx`:
- Around line 14-17: The import block in ApplicationSetApplicationsView should
be reformatted to match Prettier so lint passes; update the grouped import from
GitOpsViewType to use the standard single-line or wrapped style that Prettier
would generate, keeping APPLICATION_SET_APPLICATIONS_VIEW_SETTING_KEY and
GitOpsViewType in the same import statement.
- Around line 125-133: The list view is overriding GitOpsDataViewTable’s
internal error handling by always passing a defined activeState from
ApplicationSetApplicationsView, which causes errors to render like a normal
table. Update the activeState prop so it is only set when the view is actually
empty, and leave it undefined otherwise so GitOpsDataViewTable can fall back to
errorState when isError is true.

In `@src/gitops/components/shared/GitOpsGraphListView.scss`:
- Around line 10-18: The shared GitOpsGraphListView stylesheet is forcing a
large minimum height on both the graph and list containers, which incorrectly
affects the list/table view rendered by ApplicationSetApplicationsView. Update
GitOpsGraphListView.scss so the fixed min-height remains only on the graph panel
selector, and remove it from the list panel selector while keeping its flex
column layout intact. Verify the list path inside
gitops-graph-list-view__list-panel no longer gets the page-sized blank area
after toggling.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 62e858d1-aab1-48ac-ab31-1e10fd49d2d1

📥 Commits

Reviewing files that changed from the base of the PR and between 01e9db1 and d422cc6.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (18)
  • locales/en/plugin__gitops-plugin.json
  • locales/en/topology.json
  • locales/ja/plugin__gitops-plugin.json
  • locales/ja/topology.json
  • locales/ko/plugin__gitops-plugin.json
  • locales/ko/topology.json
  • locales/zh/plugin__gitops-plugin.json
  • locales/zh/topology.json
  • src/gitops/components/application/ApplicationResourcesTab.tsx
  • src/gitops/components/application/ApplicationResourcesToolbar.tsx
  • src/gitops/components/application/ApplicationResourcesView.tsx
  • src/gitops/components/application/ApplicationResourcesViewType.ts
  • src/gitops/components/application/graph/nodes/ApplicationNode.tsx
  • src/gitops/components/shared/ApplicationList.tsx
  • src/gitops/components/shared/ApplicationSetApplicationsView.tsx
  • src/gitops/components/shared/GitOpsGraphListView.scss
  • src/gitops/components/shared/GitOpsViewSwitcher.tsx
  • src/gitops/components/shared/GitOpsViewType.ts

import { CubesIcon } from '@patternfly/react-icons';
import { Tbody, Td, Tr } from '@patternfly/react-table';
import { useK8sModel, useUserSettings } from '@openshift-console/dynamic-plugin-sdk';
import { Flex, FlexItem, PageBody, PageSection, PageSectionVariants, Title } from '@patternfly/react-core';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Run Prettier on this import block.

ESLint reports this as a Prettier error, so it can block lint checks.

Proposed fix
-import { Flex, FlexItem, PageBody, PageSection, PageSectionVariants, Title } from '`@patternfly/react-core`';
+import {
+  Flex,
+  FlexItem,
+  PageBody,
+  PageSection,
+  PageSectionVariants,
+  Title,
+} from '`@patternfly/react-core`';
📝 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
import { Flex, FlexItem, PageBody, PageSection, PageSectionVariants, Title } from '@patternfly/react-core';
import {
Flex,
FlexItem,
PageBody,
PageSection,
PageSectionVariants,
Title,
} from '`@patternfly/react-core`';
🧰 Tools
🪛 ESLint

[error] 8-8: Replace ·Flex,·FlexItem,·PageBody,·PageSection,·PageSectionVariants,·Title· with ⏎··Flex,⏎··FlexItem,⏎··PageBody,⏎··PageSection,⏎··PageSectionVariants,⏎··Title,⏎

(prettier/prettier)

🤖 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 `@src/gitops/components/application/ApplicationResourcesTab.tsx` at line 8, The
import block in ApplicationResourcesTab should be reformatted to match
Prettier’s expected style so ESLint stops flagging it. Update the import
statement near the top of ApplicationResourcesTab.tsx using the existing React
Core import as the target, and run the file through Prettier so the formatting
is automatically corrected.

Source: Linters/SAST tools

Comment on lines +69 to +76
const argoBaseURL =
argoServer.protocol +
'://' +
argoServer.host +
'/applications/' +
obj?.metadata?.namespace +
'/' +
obj?.metadata?.name,
);
'://' +
argoServer.host +
'/applications/' +
obj?.metadata?.namespace +
'/' +
obj?.metadata?.name;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Don’t render Argo CD links/actions with an incomplete URL.

argoBaseURL is built while argoServer is still empty, so ArgoCDLink and resource actions can receive malformed URLs like :///applications/....

Proposed fix
-  const argoBaseURL =
-    argoServer.protocol +
-    '://' +
-    argoServer.host +
-    '/applications/' +
-    obj?.metadata?.namespace +
-    '/' +
-    obj?.metadata?.name;
+  const applicationNamespace = obj?.metadata?.namespace;
+  const applicationName = obj?.metadata?.name;
+  const argoBaseURL =
+    argoServer.protocol && argoServer.host && applicationNamespace && applicationName
+      ? `${argoServer.protocol}://${argoServer.host}/applications/${applicationNamespace}/${applicationName}`
+      : undefined;
-              <ArgoCDLink href={argoBaseURL} />
+              {argoBaseURL && <ArgoCDLink href={argoBaseURL} />}
-          {obj?.metadata && (
+          {obj?.metadata && argoBaseURL && (
             <ApplicationResourcesView

Also applies to: 100-112

🤖 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 `@src/gitops/components/application/ApplicationResourcesTab.tsx` around lines
69 - 76, The Argo CD URL is being constructed before the server settings are
available, so `ArgoCDLink` and the resource actions can receive a malformed
`argoBaseURL` like `:///applications/...`. Update `ApplicationResourcesTab` so
the URL is only built and passed once `argoServer` has valid protocol/host
values, and guard the render of the link/actions until the full URL is
available; apply the same check wherever `argoBaseURL` is used in the referenced
block.

RowFilterItem,
useListPageFilter,
} from '@openshift-console/dynamic-plugin-sdk';
import { EmptyState, EmptyStateBody, Flex, FlexItem, Stack, StackItem } from '@patternfly/react-core';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Run Prettier on this import block.

ESLint reports this as a Prettier error, so it can block lint checks.

Proposed fix
-import { EmptyState, EmptyStateBody, Flex, FlexItem, Stack, StackItem } from '`@patternfly/react-core`';
+import {
+  EmptyState,
+  EmptyStateBody,
+  Flex,
+  FlexItem,
+  Stack,
+  StackItem,
+} from '`@patternfly/react-core`';
📝 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
import { EmptyState, EmptyStateBody, Flex, FlexItem, Stack, StackItem } from '@patternfly/react-core';
import {
EmptyState,
EmptyStateBody,
Flex,
FlexItem,
Stack,
StackItem,
} from '`@patternfly/react-core`';
🧰 Tools
🪛 ESLint

[error] 18-18: Replace ·EmptyState,·EmptyStateBody,·Flex,·FlexItem,·Stack,·StackItem· with ⏎··EmptyState,⏎··EmptyStateBody,⏎··Flex,⏎··FlexItem,⏎··Stack,⏎··StackItem,⏎

(prettier/prettier)

🤖 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 `@src/gitops/components/application/ApplicationResourcesView.tsx` at line 18,
The import block in ApplicationResourcesView has a Prettier formatting issue
that is causing ESLint to fail. Reformat the `@patternfly/react-core` import so it
matches the project’s Prettier style, keeping the same imported symbols
(EmptyState, EmptyStateBody, Flex, FlexItem, Stack, StackItem) but adjusting the
layout/spacing to the formatter’s expected output.

Source: Linters/SAST tools

Comment on lines +161 to +164
case 'sync-wave':
aValue = a.syncWave || '';
bValue = b.syncWave || '';
break;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve valid sync wave 0.

syncWave is an optional number, and || treats 0 as missing, so wave 0 sorts/displays as -.

Proposed fix
       case 'sync-wave':
-        aValue = a.syncWave || '';
-        bValue = b.syncWave || '';
+        aValue =
+          a.syncWave ?? (direction === 'asc' ? Number.POSITIVE_INFINITY : Number.NEGATIVE_INFINITY);
+        bValue =
+          b.syncWave ?? (direction === 'asc' ? Number.POSITIVE_INFINITY : Number.NEGATIVE_INFINITY);
         break;
-        cell: <>{resource.syncWave || '-'}</>,
+        cell: <>{resource.syncWave ?? '-'}</>,

Also applies to: 272-274

🤖 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 `@src/gitops/components/application/ApplicationResourcesView.tsx` around lines
161 - 164, The sync wave handling in ApplicationResourcesView is treating valid
value 0 as missing because it uses a fallback that coerces falsy values, so wave
0 gets sorted/displayed like absent data. Update the sync-wave branches in the
comparator/rendering logic to use a nullish check instead of a falsy fallback so
0 is preserved, and apply the same fix anywhere else the syncWave value is
normalized in this component.


const { searchParams, sortBy, direction, getSortParams } =
useGitOpsDataViewSort(columnSortConfig);
const { searchParams, sortBy, direction, getSortParams } = useGitOpsDataViewSort(columnSortConfig);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Apply Prettier formatting to unblock lint.

Proposed fix
-  const { searchParams, sortBy, direction, getSortParams } = useGitOpsDataViewSort(columnSortConfig);
+  const { searchParams, sortBy, direction, getSortParams } = useGitOpsDataViewSort(
+    columnSortConfig,
+  );
📝 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
const { searchParams, sortBy, direction, getSortParams } = useGitOpsDataViewSort(columnSortConfig);
const { searchParams, sortBy, direction, getSortParams } = useGitOpsDataViewSort(
columnSortConfig,
);
🧰 Tools
🪛 ESLint

[error] 114-114: Insert ⏎···

(prettier/prettier)

🤖 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 `@src/gitops/components/shared/ApplicationList.tsx` at line 114, The
destructuring assignment in ApplicationList/useGitOpsDataViewSort needs Prettier
formatting to satisfy lint. Reformat the declaration so the imported values from
useGitOpsDataViewSort(columnSortConfig) follow the project’s standard line
wrapping and spacing, keeping the same symbols searchParams, sortBy, direction,
and getSortParams intact.

Source: Linters/SAST tools

<ApplicationSetApplicationsView
applicationSet={appset as ApplicationSetKind}
ownedApps={ownedApps as ApplicationKind[]}
filteredApplications={filteredData as ApplicationKind[]}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Pass the search-filtered applications to the graph view.

Line 256 sends filteredData, while rows and isEmpty use filteredBySearch. In appset graph mode, URL search filters can be ignored and show applications that the list view hides.

Proposed fix
-            filteredApplications={filteredData as ApplicationKind[]}
+            filteredApplications={filteredBySearch as ApplicationKind[]}
📝 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
filteredApplications={filteredData as ApplicationKind[]}
filteredApplications={filteredBySearch as ApplicationKind[]}
🤖 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 `@src/gitops/components/shared/ApplicationList.tsx` at line 256, The graph view
is still receiving the unsearched filteredData set, so URL search filters can be
ignored in appset graph mode. Update the ApplicationList render path to pass the
same search-filtered collection used by rows and isEmpty, namely
filteredBySearch, into the graph view via filteredApplications. Keep the fix
localized to the ApplicationList component so the list and graph stay in sync.

Comment on lines +14 to +17
import {
APPLICATION_SET_APPLICATIONS_VIEW_SETTING_KEY,
GitOpsViewType,
} from './GitOpsViewType';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Apply Prettier formatting to unblock lint.

Proposed fix
-import {
-  APPLICATION_SET_APPLICATIONS_VIEW_SETTING_KEY,
-  GitOpsViewType,
-} from './GitOpsViewType';
+import { APPLICATION_SET_APPLICATIONS_VIEW_SETTING_KEY, GitOpsViewType } from './GitOpsViewType';
📝 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
import {
APPLICATION_SET_APPLICATIONS_VIEW_SETTING_KEY,
GitOpsViewType,
} from './GitOpsViewType';
import { APPLICATION_SET_APPLICATIONS_VIEW_SETTING_KEY, GitOpsViewType } from './GitOpsViewType';
🧰 Tools
🪛 ESLint

[error] 14-17: Replace ⏎··APPLICATION_SET_APPLICATIONS_VIEW_SETTING_KEY,⏎··GitOpsViewType,⏎ with ·APPLICATION_SET_APPLICATIONS_VIEW_SETTING_KEY,·GitOpsViewType·

(prettier/prettier)

🤖 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 `@src/gitops/components/shared/ApplicationSetApplicationsView.tsx` around lines
14 - 17, The import block in ApplicationSetApplicationsView should be
reformatted to match Prettier so lint passes; update the grouped import from
GitOpsViewType to use the standard single-line or wrapped style that Prettier
would generate, keeping APPLICATION_SET_APPLICATIONS_VIEW_SETTING_KEY and
GitOpsViewType in the same import statement.

Source: Linters/SAST tools

Comment on lines +125 to +133
<GitOpsDataViewTable
columns={columns}
rows={rows}
isEmpty={isEmpty}
emptyState={emptyState}
errorState={errorState}
isError={isError}
activeState={isEmpty ? DataViewState.empty : null}
/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve the error state in list view.

Line 132 always passes a defined activeState; null overrides GitOpsDataViewTable’s built-in error resolution, so load errors render as a normal table instead of errorState.

Proposed fix
-                activeState={isEmpty ? DataViewState.empty : null}
+                activeState={isError ? DataViewState.error : isEmpty ? DataViewState.empty : null}
📝 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
<GitOpsDataViewTable
columns={columns}
rows={rows}
isEmpty={isEmpty}
emptyState={emptyState}
errorState={errorState}
isError={isError}
activeState={isEmpty ? DataViewState.empty : null}
/>
<GitOpsDataViewTable
columns={columns}
rows={rows}
isEmpty={isEmpty}
emptyState={emptyState}
errorState={errorState}
isError={isError}
activeState={isError ? DataViewState.error : isEmpty ? DataViewState.empty : null}
/>
🤖 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 `@src/gitops/components/shared/ApplicationSetApplicationsView.tsx` around lines
125 - 133, The list view is overriding GitOpsDataViewTable’s internal error
handling by always passing a defined activeState from
ApplicationSetApplicationsView, which causes errors to render like a normal
table. Update the activeState prop so it is only set when the view is actually
empty, and leave it undefined otherwise so GitOpsDataViewTable can fall back to
errorState when isError is true.

Comment on lines +10 to +18
&__panel,
&__list-panel {
min-height: 1000px;
}

&__list-panel {
display: flex;
flex-direction: column;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Don't force the graph height onto the list panel.

src/gitops/components/shared/ApplicationSetApplicationsView.tsx:91-102 renders the table path inside gitops-graph-list-view__list-panel. Giving that container min-height: 1000px makes short or empty list views render with a page-sized blank region after every toggle. Keep the fixed height on the graph panel only.

Suggested fix
-  &__panel,
-  &__list-panel {
+  &__panel {
     min-height: 1000px;
   }
 
   &__list-panel {
+    min-height: 0;
     display: flex;
     flex-direction: column;
   }

As per path instructions, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."

📝 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
&__panel,
&__list-panel {
min-height: 1000px;
}
&__list-panel {
display: flex;
flex-direction: column;
}
&__panel {
min-height: 1000px;
}
&__list-panel {
min-height: 0;
display: flex;
flex-direction: column;
}
🤖 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 `@src/gitops/components/shared/GitOpsGraphListView.scss` around lines 10 - 18,
The shared GitOpsGraphListView stylesheet is forcing a large minimum height on
both the graph and list containers, which incorrectly affects the list/table
view rendered by ApplicationSetApplicationsView. Update GitOpsGraphListView.scss
so the fixed min-height remains only on the graph panel selector, and remove it
from the list panel selector while keeping its flex column layout intact. Verify
the list path inside gitops-graph-list-view__list-panel no longer gets the
page-sized blank area after toggling.

Source: Path instructions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants