chore(KFLUXUI-1004): new component list page#707
Open
janaki29 wants to merge 1 commit intokonflux-ci:mainfrom
Open
chore(KFLUXUI-1004): new component list page#707janaki29 wants to merge 1 commit intokonflux-ci:mainfrom
janaki29 wants to merge 1 commit intokonflux-ci:mainfrom
Conversation
janaki29
requested review from
marcin-michal and
milantaky
and removed request for
a team
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Contributor
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
ComponentList core src/components/ComponentList/ComponentList.tsx |
New main list component: fetches components and latest build pipelines, merges data, applies name filter, provides toolbar, empty/getting-started states, and renders Table. Exports COMPONENTS_LIST_GS_LOCAL_STORAGE_KEY and default ComponentList. |
Header & Row src/components/ComponentList/ComponentListHeader.tsx, src/components/ComponentList/ComponentListRow.tsx |
New header column class mapping and column definitions; row renderer with name link (uses COMPONENT_DETAILS_V2_PATH), optional GitRepoLink, image preview via ImageUrlDisplay, versions count, and action menu. |
Styling src/components/ComponentList/ComponentList.scss |
Adds SCSS for layout, image block, build-completion layout, actions alignment, row actions, and empty-state spacing. |
Routing src/routes/page-routes/components-page.tsx |
Replaces lazy route with a ComponentsListRoute that renders ComponentList wrapped in FilterContextProvider (filters: name). |
Types src/types/component.ts |
Adds ComponentSourceVersion type and versions?: ComponentSourceVersion[] to ComponentSource. |
Tests src/components/ComponentList/__tests__/* src/components/ComponentList/__tests__/ComponentList.spec.tsx, .../ComponentListHeader.spec.tsx, .../ComponentListRow.spec.tsx |
New unit tests covering header columns, row rendering (links, versions, image, actions), ComponentList loading/error/empty/getting-started flows, integration with hooks and localStorage key. |
Sequence Diagram
sequenceDiagram
participant UI as ComponentList
participant ComponentsHook as useAllComponents
participant PipelinesHook as useLatestPushBuildPipelines
participant FilterCtx as FilterContext
participant Table as Table
UI->>ComponentsHook: request components(namespace)
ComponentsHook-->>UI: components[]
UI->>PipelinesHook: request latest pipelines(component names, namespace)
PipelinesHook-->>UI: pipelineRuns[]
UI->>UI: merge components with latest pipelineRuns
UI->>FilterCtx: read name filter
FilterCtx-->>UI: filter value
UI->>UI: compute filteredComponents
UI->>Table: render header + rows (filteredComponents), pass loading/error states
Table-->>UI: rendered table
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- KFLUXUI-1056 [Snapshot Details] update component name link to open the Component Details page #701: Updates component detail routing used for component name links (ComponentListRow uses COMPONENT_DETAILS_V2_PATH).
- feat(KFLUXUI-690): Adds a feature flag and new Components page #577: Related changes to the Components page routing and list UI integration.
- Support proxy image url for private image repo #563: Introduced or modified ImageUrlDisplay used by ComponentListRow for image previews.
Suggested labels
ok-to-test
Suggested reviewers
- sahil143
- JoaoPedroPP
- abhinandan13jan
- StanislavJochman
- marcin-michal
- milantaky
Poem
A list of components lines the view,
Hooks fetch the bits, the table grew.
Filters whisper, images gleam,
Rows link onward to the dream.
🎉 New UI stitched into the queue.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately describes the main change: introducing a new component list page with all supporting UI components, types, and tests. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
Warning
There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.
🔧 Stylelint (17.3.0)
src/components/ComponentList/ComponentList.scss
ConfigurationError: Could not find "stylelint-config-recommended-scss". Do you need to install the package or use the "configBasedir" option?
at getModulePath (file:///usr/local/lib/node_modules/stylelint/lib/utils/getModulePath.mjs:29:9)
at loadExtendedConfig (file:///usr/local/lib/node_modules/stylelint/lib/augmentConfig.mjs:241:21)
at extendConfig (file:///usr/local/lib/node_modules/stylelint/lib/augmentConfig.mjs:208:25)
at augmentConfigBasic (file:///usr/local/lib/node_modules/stylelint/lib/augmentConfig.mjs:73:26)
at augmentConfigFull (file:///usr/local/lib/node_modules/stylelint/lib/augmentConfig.mjs:126:30)
at getConfigForFile (file:///usr/local/lib/node_modules/stylelint/lib/getConfigForFile.mjs:102:32)
at async resolveOptionValue (file:///usr/local/lib/node_modules/stylelint/lib/utils/resolveOptionValue.mjs:27:24)
at async standalone (file:///usr/local/lib/node_modules/stylelint/lib/standalone.mjs:202:19)
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
janaki29
requested review from
JoaoPedroPP,
StanislavJochman,
abhinandan13jan,
rrosatti and
sahil143
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/components/ComponentList/ComponentList.tsx`:
- Around line 92-107: NoDataEmptyMessage renders a ButtonWithAccessTooltip
without any click behavior; add an onClick prop to the ButtonWithAccessTooltip
in the NoDataEmptyMessage component so the button performs the intended action
(e.g., open the create-component flow). Wire the onClick to the existing
create-flow handler (call the project’s openCreateComponentModal or
onAddComponent prop if available) or use the router hook (e.g., useNavigate) to
navigate to the component creation route (e.g., "/components/new"); ensure you
import any hook or handler and keep the tooltip/variant props intact.
- Around line 59-63: The hook call passes an empty applicationName so
useLatestPushBuildPipelines filters by PipelineRunLabel.APPLICATION = '' and
returns nothing; change the hook to treat applicationName as optional (or add a
namespace-scoped variant) so when applicationName is empty/undefined it skips
the application label filter and does not call useApplication(namespace, ''),
e.g., update useLatestPushBuildPipelines (or create
useLatestPushBuildPipelinesForNamespace) to conditionally apply the
PipelineRunLabel.APPLICATION filter only when a non-empty applicationName is
provided and fetch pipelines by namespace otherwise.
In `@src/components/ComponentList/ComponentListHeader.tsx`:
- Around line 1-7: componentsTableColumnClasses currently assigns PatternFly
width modifiers whose numeric values add up to 145, causing layout overflow;
update the width tokens on the object (keys: component, gitRepository,
imageRegistry, componentVersions, kebab) so their pf-m-width-N values sum to
roughly 100 (for example by reducing one or more of
component/gitRepository/imageRegistry values) and keep any extra classes like
wrap-column and component-list-view__actions unchanged; ensure the new
proportions preserve visual balance and test the table at common breakpoints.
In `@src/components/ComponentList/ComponentListRow.tsx`:
- Around line 64-70: The container around ImageUrlDisplay in ComponentListRow
currently uses only textOverflow: 'ellipsis' which doesn't work alone; replace
this raw <div> with PatternFly's Truncate component (import Truncate from
'@patternfly/react-core') and wrap the ImageUrlDisplay so long URLs are
truncated per guidelines, or if you must keep a container, add overflow:
'hidden' and whiteSpace: 'nowrap' together with textOverflow: 'ellipsis'; also
remove the inline style and update imports to include Truncate (or add the CSS
class) so the change targets ComponentListRow and the ImageUrlDisplay wrapper.
In `@src/types/component.ts`:
- Line 24: The ComponentSource type's versions property is declared as number
but runtime code in ComponentListRow.tsx (which inspects
ComponentSource.versions) handles arrays and strings too; update the type
declaration for versions in ComponentSource to reflect actual runtime shapes
(e.g., versions?: number | string | unknown[]) or a more precise union (e.g.,
number | string | Array<number|string>), OR if the API should only return
numbers, change the runtime handling in ComponentListRow.tsx to only accept
numbers and fail/normalize other types; reference the symbol ComponentSource and
the versions property when making the change so the type and usage are
consistent.
🧹 Nitpick comments (6)
src/routes/page-routes/components-page.tsx (1)
2-2: Use path aliases for new imports.New imports on lines 2 and 8 use relative paths. Per coding guidelines, use configured path aliases (
~/components).-import ComponentList from '../../components/ComponentList/ComponentList'; +import ComponentList from '~/components/ComponentList/ComponentList';-import { FilterContextProvider } from '../../components/Filter/generic/FilterContext'; +import { FilterContextProvider } from '~/components/Filter/generic/FilterContext';As per coding guidelines: "Use absolute imports with configured path aliases:
~/components,~/types,~/k8s,~/utils,~/models,@routes"src/components/ComponentList/ComponentList.scss (1)
1-31: Minor BEM naming inconsistency.Two different block names are used:
.component-list-view(line 1) and.component-list(line 29). Consider unifying under one block name for consistency.src/components/ComponentList/ComponentList.tsx (2)
166-166: Raw<div>used for layout.Per coding guidelines, use PatternFly layout components instead of raw HTML layout tags.
17-24: Multiple relative imports should use path aliases.Lines 17–24 use relative paths (
../../assets/,../../consts/,../../hooks/,../../shared,../../shared/...,../../types). Per coding guidelines, use configured path aliases like~/hooks,~/shared,~/types, etc.As per coding guidelines: "Use absolute imports with configured path aliases:
~/components,~/types,~/k8s,~/utils,~/models,@routes"src/components/ComponentList/ComponentListRow.tsx (2)
27-38: Defensive checks forArrayandstringare dead code given the current type.
versionsis typed asnumber | undefinedinComponentSource. The array and string branches (lines 28-36) are unreachable. If the API can actually return these types, fix the type definition incomponent.ts; otherwise, remove the dead branches.
4-13: Use path aliases for imports.Several imports use relative paths. Per coding guidelines, use path aliases (
~/hooks,~/shared,~/types,~/utils,~/components,@routes).As per coding guidelines: "Use absolute imports with configured path aliases:
~/components,~/types,~/k8s,~/utils,~/models,@routes"
Comment on lines
+92
to
+107
| const NoDataEmptyMessage = () => ( | ||
| <AppEmptyState | ||
| className="component-list__empty-state" | ||
| emptyStateImg={emptyStateImgUrl} | ||
| title="" | ||
| > | ||
| <EmptyStateBody> | ||
| A component is an image built from source code in a repository. | ||
| <br /> | ||
| To get started, add a component. | ||
| </EmptyStateBody> | ||
| <ButtonWithAccessTooltip variant="primary" tooltip="You don't have access to add a component"> | ||
| Add component | ||
| </ButtonWithAccessTooltip> | ||
| </AppEmptyState> | ||
| ); |
Contributor
There was a problem hiding this comment.
NoDataEmptyMessage: "Add component" button has no onClick handler.
The button renders but doesn't navigate anywhere or trigger any action. This is a non-functional UI element that will confuse users.
🤖 Prompt for AI Agents
In `@src/components/ComponentList/ComponentList.tsx` around lines 92 - 107,
NoDataEmptyMessage renders a ButtonWithAccessTooltip without any click behavior;
add an onClick prop to the ButtonWithAccessTooltip in the NoDataEmptyMessage
component so the button performs the intended action (e.g., open the
create-component flow). Wire the onClick to the existing create-flow handler
(call the project’s openCreateComponentModal or onAddComponent prop if
available) or use the router hook (e.g., useNavigate) to navigate to the
component creation route (e.g., "/components/new"); ensure you import any hook
or handler and keep the tooltip/variant props intact.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #707 +/- ##
==========================================
- Coverage 86.63% 86.59% -0.04%
==========================================
Files 764 767 +3
Lines 58225 58542 +317
Branches 5658 6903 +1245
==========================================
+ Hits 50441 50696 +255
- Misses 7603 7616 +13
- Partials 181 230 +49
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 73 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
janaki29
force-pushed
the
component-list
branch
from
3d5b121 to
042993f
Compare
rrosatti
reviewed
Feb 17, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
src/components/ComponentList/ComponentList.tsx (3)
38-44:statusfilter is parsed but never settable from the UI.The
filtersmemo extracts bothnameandstatusfrom the filter context (lines 40–41), but the toolbar (lines 139–147) only provides a text input forname. ThestatusFiltervariable at line 44 is initialized but never populated through any UI control. This is dead plumbing — it doesn't break anything, but it adds unnecessary complexity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/ComponentList.tsx` around lines 38 - 44, The code parses a status filter into the local filters object but no UI control sets it, so remove the unused plumbing: update the useDeepCompareMemoize call in ComponentList to only include name (remove status), remove status from the destructuring (drop statusFilter), and adjust any subsequent code that references statusFilter; alternatively if you intend to support status via the toolbar, add a UI control that updates FilterContext using setFilters/onClearFilters to set status (ensure updates flow through useDeepCompareMemoize and the { name: nameFilter } destructuring).
11-11: Unused imports:capitalizeandpipelineRunStatus.If the dead status filter code (lines 78–84) is removed as suggested,
capitalizefromlodash-es(line 11) andpipelineRunStatusfrompipeline-utils(line 17) become unused imports.Also applies to: 17-17
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/ComponentList.tsx` at line 11, The imports capitalize (from 'lodash-es') and pipelineRunStatus (from 'pipeline-utils') are now unused after removing the dead status filter; remove the unused import statements referencing capitalize and pipelineRunStatus from ComponentList (delete the capitalize import and the pipelineRunStatus import) so the file has no unused imports, or alternatively reintroduce usage where appropriate if you intend to keep them.
1-31: Import style inconsistency: mix of absolute and relative paths.Lines 12–16 correctly use
~/aliases, but lines 18–30 fall back to relative../../paths. Same issue as inComponentListRow.tsx.♻️ Suggested import cleanup
-import emptyStateImgUrl from '../../assets/Components.svg'; -import pipelineImg from '../../assets/Pipeline.svg'; -import { PipelineRunLabel } from '../../consts/pipelinerun'; -import { useLatestPushBuildPipelines } from '../../hooks/useLatestPushBuildPipelines'; -import { Table, useDeepCompareMemoize } from '../../shared'; -import AppEmptyState from '../../shared/components/empty-state/AppEmptyState'; -import FilteredEmptyState from '../../shared/components/empty-state/FilteredEmptyState'; -import { useNamespace } from '../../shared/providers/Namespace/useNamespaceInfo'; -import { ComponentKind } from '../../types'; -import { ButtonWithAccessTooltip } from '../ButtonWithAccessTooltip'; -import { GettingStartedCard } from '../GettingStartedCard/GettingStartedCard'; +import emptyStateImgUrl from '~/assets/Components.svg'; +import pipelineImg from '~/assets/Pipeline.svg'; +import { PipelineRunLabel } from '~/consts/pipelinerun'; +import { useLatestPushBuildPipelines } from '~/hooks/useLatestPushBuildPipelines'; +import { Table, useDeepCompareMemoize } from '~/shared'; +import AppEmptyState from '~/shared/components/empty-state/AppEmptyState'; +import FilteredEmptyState from '~/shared/components/empty-state/FilteredEmptyState'; +import { useNamespace } from '~/shared/providers/Namespace/useNamespaceInfo'; +import { ComponentKind } from '~/types'; +import { ButtonWithAccessTooltip } from '~/components/ButtonWithAccessTooltip'; +import { GettingStartedCard } from '~/components/GettingStartedCard/GettingStartedCard';As per coding guidelines: "Use absolute imports with configured path aliases."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/ComponentList.tsx` around lines 1 - 31, The file mixes alias imports and relative imports; replace the relative '../../' and './' imports with the configured absolute alias form so all modules use the same import style (e.g., change imports for emptyStateImgUrl, pipelineImg, PipelineRunLabel, useLatestPushBuildPipelines, Table, useDeepCompareMemoize, AppEmptyState, FilteredEmptyState, useNamespace, ButtonWithAccessTooltip, GettingStartedCard, ComponentsListHeader, ComponentsListRow, and ComponentList.scss to use the ~/ alias or other project path aliases), ensuring each referenced symbol (e.g., useLatestPushBuildPipelines, PipelineRunLabel, ComponentsListRow) resolves via the tsconfig/webpack path mapping and updating any sibling/styles imports to their aliased equivalents if configured.src/components/ComponentList/ComponentListRow.tsx (3)
67-71: Raw<div>for action menu wrapper.The
<div className="component-list-view__row-actions">uses raw HTML. Consider replacing it with a PatternFlyFlexItemor ensuring the custom class is necessary. If it's only for alignment, a PatternFly layout component would be more consistent.As per coding guidelines: "NEVER use raw HTML layout tags (
<div>,<section>,<header>) or custom CSS layouts."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/ComponentListRow.tsx` around lines 67 - 71, The wrapper currently uses a raw <div className="component-list-view__row-actions"> around ActionMenu; replace this with a PatternFly layout component (e.g., FlexItem) to comply with the guideline or, if the custom class is required, move its layout rules into a PatternFly wrapper. Specifically, in ComponentListRow.tsx update the TableData cell that renders ActionMenu to use PatternFly's Flex/FlexItem (import from `@patternfly/react-core`), apply the existing className to the FlexItem if you need the same styling, or remove the custom CSS and use Flex/FlexItem props for alignment so the ActionMenu is wrapped by a PatternFly component instead of a raw div; ensure ActionMenu and componentsTableColumnClasses.kebab usage remain unchanged.
50-59: Raw<div>wrapper aroundImageUrlDisplay.Per coding guidelines, raw HTML layout tags should not be used — prefer PatternFly layout components. The wrapping
<div>on line 52 doesn't add meaningful structure. If it's needed for overflow/truncation, consider using PatternFly'sTruncatecomponent or at minimum aFlexItem.♻️ Remove unnecessary div wrapper
<TableData className={componentsTableColumnClasses.imageRegistry}> {latestImage && ( - <div> <ImageUrlDisplay imageUrl={latestImage} namespace={component.metadata.namespace} componentName={component.metadata.name} /> - </div> )} </TableData>As per coding guidelines: "NEVER use raw HTML layout tags (
<div>,<section>,<header>) or custom CSS layouts."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/ComponentListRow.tsx` around lines 50 - 59, The raw <div> wrapper around ImageUrlDisplay in ComponentListRow should be removed and replaced with a PatternFly layout/truncation component; locate the JSX inside TableData (componentsTableColumnClasses.imageRegistry) and replace the div surrounding <ImageUrlDisplay ... /> with a PatternFly Truncate or a FlexItem (or other PF layout component) to handle overflow/truncation, ensuring props namespace and componentName are preserved and no extra DOM wrapper remains.
1-13: Inconsistent import style: mix of relative and absolute paths.Line 3 uses the
~/shared/alias, but lines 4–12 use relative paths (../../hooks/,../../shared/,../../types, etc.). The codebase convention requires absolute imports with configured path aliases (~/components,~/types,~/hooks,~/utils,~/shared,@routes).♻️ Suggested import cleanup
import * as React from 'react'; import { Link } from 'react-router-dom'; import ActionMenu from '~/shared/components/action-menu/ActionMenu'; -import { PacStatesForComponents } from '../../hooks/usePACStatesForComponents'; -import { COMPONENT_DETAILS_V2_PATH } from '../../routes/paths'; -import { RowFunctionArgs, TableData } from '../../shared'; -import { ImageUrlDisplay } from '../../shared/components/image-display'; -import { useNamespace } from '../../shared/providers/Namespace/useNamespaceInfo'; -import { ComponentKind, PipelineRunKind } from '../../types'; -import { getLastestImage } from '../../utils/component-utils'; -import { useComponentActions } from '../Components/component-actions'; -import GitRepoLink from '../GitLink/GitRepoLink'; +import { PacStatesForComponents } from '~/hooks/usePACStatesForComponents'; +import { COMPONENT_DETAILS_V2_PATH } from '@routes/paths'; +import { RowFunctionArgs, TableData } from '~/shared'; +import { ImageUrlDisplay } from '~/shared/components/image-display'; +import { useNamespace } from '~/shared/providers/Namespace/useNamespaceInfo'; +import { ComponentKind, PipelineRunKind } from '~/types'; +import { getLastestImage } from '~/utils/component-utils'; +import { useComponentActions } from '~/components/Components/component-actions'; +import GitRepoLink from '~/components/GitLink/GitRepoLink';As per coding guidelines: "Use absolute imports with configured path aliases:
~/components,~/types,~/k8s,~/utils,~/models,@routes."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/ComponentListRow.tsx` around lines 1 - 13, The imports in ComponentListRow.tsx mix the ~/ alias with relative paths; update all imports to use the project's configured path aliases (e.g., replace ../../hooks/usePACStatesForComponents with ~/hooks/usePACStatesForComponents, ../../shared with ~/shared, ../../types with ~/types, ../../utils/component-utils with ~/utils/component-utils, ../Components/component-actions with ~/components/component-actions, ../GitLink/GitRepoLink with ~/components/GitLink/GitRepoLink, and ../../routes/paths with `@routes/paths`) so every symbol (PacStatesForComponents, COMPONENT_DETAILS_V2_PATH, RowFunctionArgs, TableData, ImageUrlDisplay, useNamespace, ComponentKind, PipelineRunKind, getLastestImage, useComponentActions, GitRepoLink, componentsTableColumnClasses) is imported via the alias convention; run project lint/TS build to verify no path errors.src/components/ComponentList/__tests__/ComponentList.spec.tsx (1)
4-4: Consider usingrenderWithQueryClientAndRouterinstead ofrenderWithQueryClient.The
ComponentListcomponent tree renders PatternFly and shared components that may internally depend on Router context (e.g., navigation links within sub-components,GettingStartedCard, etc.). WhileComponentListRowis mocked, other unmocked descendants might still need a Router provider. The coding guidelines recommendrenderWithQueryClientAndRouterfrom~/unit-test-utils/rendering-utilswhen components use React Query and Router context.♻️ Suggested change
-import { renderWithQueryClient } from '~/unit-test-utils/mock-react-query'; +import { renderWithQueryClientAndRouter } from '~/unit-test-utils/rendering-utils';const renderComponentList = () => - renderWithQueryClient( + renderWithQueryClientAndRouter( <FilterContextProvider filterParams={['name', 'status']}> <ComponentList /> </FilterContextProvider>, );As per coding guidelines: "Use
renderWithQueryClientAndRouterfrom~/unit-test-utils/rendering-utilsfor rendering components with React Query and Router context."Also applies to: 52-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/__tests__/ComponentList.spec.tsx` at line 4, The test imports and uses renderWithQueryClient but the ComponentList tree needs Router context; replace usage with renderWithQueryClientAndRouter from ~/unit-test-utils/rendering-utils by updating the import (swap renderWithQueryClient for renderWithQueryClientAndRouter) and any render calls (e.g., calls to renderWithQueryClient(...)) to call renderWithQueryClientAndRouter(...) so the component receives both React Query and Router providers (update references in ComponentList.spec.tsx, including the imports at top and the 52-57 render calls).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ComponentList/__tests__/ComponentList.spec.tsx`:
- Around line 138-143: The test is asserting that useLatestPushBuildPipelines is
called with an empty applicationName (''), which mirrors the bug; update the
test so it supplies a real application name on the component or via the render
helper and assert that useLatestPushBuildPipelinesMock is called with that
applicationName instead of ''. Concretely, modify the mocked component created
by createMockComponent('c1') or the useAllComponentsMock return to include
applicationName (e.g. 'my-app') and change the expectation in the test (the call
asserted in the it block) from ('test-ns', '', ['c1']) to ('test-ns', 'my-app',
['c1']); keep references to renderComponentList, useAllComponentsMock and
useLatestPushBuildPipelinesMock when making the change.
- Around line 8-14: The test imports useLatestPushBuildPipelines via a relative
path but the Jest mock uses the absolute ~/ alias; update the import in
ComponentList.spec.tsx to use the absolute alias so it matches the mocked
module—change the current relative import of useLatestPushBuildPipelines to
import { useLatestPushBuildPipelines } from
'~/hooks/useLatestPushBuildPipelines'; so the mocked module and the tested
import resolve identically.
In `@src/components/ComponentList/ComponentList.tsx`:
- Around line 75-88: The current useMemo for filteredComponents uses a
case-sensitive name match and contains dead status-filter logic; update the
predicate in the filteredComponents memo to compare component.metadata.name and
nameFilter in lower-case (e.g. .toLowerCase()) so name filtering is
case-insensitive, and remove the unused statusFilter path: drop compStatus, the
pipelineRunStatus(...) call, and the capitalize(compStatus) check (and any
references to statusFilter) since the toolbar does not provide a status UI;
ensure you also remove statusFilter from the useMemo dependency array if no
longer used.
---
Duplicate comments:
In `@src/components/ComponentList/ComponentList.tsx`:
- Around line 57-61: The hook useLatestPushBuildPipelines is being invoked with
an empty string for applicationName which causes its internal filter on
PipelineRunLabel.APPLICATION to never match any pipelines; update the call in
ComponentList to pass undefined (or omit the applicationName argument) instead
of '' so the hook won't filter by application label when no application is
provided, ensuring latestBuildPipelineRun can be populated for components.
- Around line 90-105: The "Add component" button in the NoDataEmptyMessage
component is missing an onClick handler, so ButtonWithAccessTooltip (rendered
inside NoDataEmptyMessage) does nothing when clicked; add an onClick prop that
triggers the intended action (e.g., navigate to the component creation route or
call the existing add-component modal function). Locate NoDataEmptyMessage and
modify the ButtonWithAccessTooltip element to call the appropriate handler (for
example a function named handleAddComponent, or use
history.push('/components/new') or invoke openAddComponentModal()), and ensure
the tooltip/access wrapper still receives any required props and the button
respects access/disabled states. Ensure the handler is defined in the same
component or passed in as a prop so tests and other callers can stub it.
In `@src/components/ComponentList/ComponentListHeader.tsx`:
- Around line 1-7: The componentsTableColumnClasses object currently assigns
PatternFly width modifiers that sum to 145%, causing layout overflow; update the
values in componentsTableColumnClasses (keys: component, gitRepository,
imageRegistry, componentVersions, kebab) so the pf-m-width-* modifiers total
~100% (for example redistribute widths across those keys to sum to 100, e.g.,
lower gitRepository/imageRegistry/kebab values) and verify the rendered table no
longer overflows.
---
Nitpick comments:
In `@src/components/ComponentList/__tests__/ComponentList.spec.tsx`:
- Line 4: The test imports and uses renderWithQueryClient but the ComponentList
tree needs Router context; replace usage with renderWithQueryClientAndRouter
from ~/unit-test-utils/rendering-utils by updating the import (swap
renderWithQueryClient for renderWithQueryClientAndRouter) and any render calls
(e.g., calls to renderWithQueryClient(...)) to call
renderWithQueryClientAndRouter(...) so the component receives both React Query
and Router providers (update references in ComponentList.spec.tsx, including the
imports at top and the 52-57 render calls).
In `@src/components/ComponentList/ComponentList.tsx`:
- Around line 38-44: The code parses a status filter into the local filters
object but no UI control sets it, so remove the unused plumbing: update the
useDeepCompareMemoize call in ComponentList to only include name (remove
status), remove status from the destructuring (drop statusFilter), and adjust
any subsequent code that references statusFilter; alternatively if you intend to
support status via the toolbar, add a UI control that updates FilterContext
using setFilters/onClearFilters to set status (ensure updates flow through
useDeepCompareMemoize and the { name: nameFilter } destructuring).
- Line 11: The imports capitalize (from 'lodash-es') and pipelineRunStatus (from
'pipeline-utils') are now unused after removing the dead status filter; remove
the unused import statements referencing capitalize and pipelineRunStatus from
ComponentList (delete the capitalize import and the pipelineRunStatus import) so
the file has no unused imports, or alternatively reintroduce usage where
appropriate if you intend to keep them.
- Around line 1-31: The file mixes alias imports and relative imports; replace
the relative '../../' and './' imports with the configured absolute alias form
so all modules use the same import style (e.g., change imports for
emptyStateImgUrl, pipelineImg, PipelineRunLabel, useLatestPushBuildPipelines,
Table, useDeepCompareMemoize, AppEmptyState, FilteredEmptyState, useNamespace,
ButtonWithAccessTooltip, GettingStartedCard, ComponentsListHeader,
ComponentsListRow, and ComponentList.scss to use the ~/ alias or other project
path aliases), ensuring each referenced symbol (e.g.,
useLatestPushBuildPipelines, PipelineRunLabel, ComponentsListRow) resolves via
the tsconfig/webpack path mapping and updating any sibling/styles imports to
their aliased equivalents if configured.
In `@src/components/ComponentList/ComponentListRow.tsx`:
- Around line 67-71: The wrapper currently uses a raw <div
className="component-list-view__row-actions"> around ActionMenu; replace this
with a PatternFly layout component (e.g., FlexItem) to comply with the guideline
or, if the custom class is required, move its layout rules into a PatternFly
wrapper. Specifically, in ComponentListRow.tsx update the TableData cell that
renders ActionMenu to use PatternFly's Flex/FlexItem (import from
`@patternfly/react-core`), apply the existing className to the FlexItem if you
need the same styling, or remove the custom CSS and use Flex/FlexItem props for
alignment so the ActionMenu is wrapped by a PatternFly component instead of a
raw div; ensure ActionMenu and componentsTableColumnClasses.kebab usage remain
unchanged.
- Around line 50-59: The raw <div> wrapper around ImageUrlDisplay in
ComponentListRow should be removed and replaced with a PatternFly
layout/truncation component; locate the JSX inside TableData
(componentsTableColumnClasses.imageRegistry) and replace the div surrounding
<ImageUrlDisplay ... /> with a PatternFly Truncate or a FlexItem (or other PF
layout component) to handle overflow/truncation, ensuring props namespace and
componentName are preserved and no extra DOM wrapper remains.
- Around line 1-13: The imports in ComponentListRow.tsx mix the ~/ alias with
relative paths; update all imports to use the project's configured path aliases
(e.g., replace ../../hooks/usePACStatesForComponents with
~/hooks/usePACStatesForComponents, ../../shared with ~/shared, ../../types with
~/types, ../../utils/component-utils with ~/utils/component-utils,
../Components/component-actions with ~/components/component-actions,
../GitLink/GitRepoLink with ~/components/GitLink/GitRepoLink, and
../../routes/paths with `@routes/paths`) so every symbol (PacStatesForComponents,
COMPONENT_DETAILS_V2_PATH, RowFunctionArgs, TableData, ImageUrlDisplay,
useNamespace, ComponentKind, PipelineRunKind, getLastestImage,
useComponentActions, GitRepoLink, componentsTableColumnClasses) is imported via
the alias convention; run project lint/TS build to verify no path errors.
5b93641 to
782edf5
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/components/ComponentList/ComponentListRow.tsx (1)
1-14: Mixed import path styles.Same as
ComponentList.tsx:~/aliases (lines 3, 5) mixed with../../relative imports (lines 4, 6–13). Prefer consistent~/path aliases per coding guidelines.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/ComponentListRow.tsx` around lines 1 - 14, The file mixes '~/'-aliased imports and '../../' relative imports; update the relative imports used in this module (symbols: PacStatesForComponents, COMPONENT_DETAILS_V2_PATH, RowFunctionArgs, TableData, ImageUrlDisplay, useNamespace, ComponentKind, PipelineRunKind, getLastestImage, useComponentActions, GitRepoLink, componentsTableColumnClasses) to use the project's '~/'-alias form so all imports in ComponentListRow.tsx are consistent with ComponentList.tsx; verify import paths resolve with the existing tsconfig/webpack alias and run the linter/build to confirm no unresolved imports remain.src/components/ComponentList/ComponentList.tsx (2)
33-178: New feature not wrapped with a feature flag.Per coding guidelines, new features should be wrapped with client-side feature flags using
IfFeaturefrom~/feature-flags/hooks. WhileFeatureFlagIndicatoris rendered on line 149 for display purposes, the component itself isn't gated behind a feature flag. This should be wrapped at the route or component level withIfFeature(and a corresponding flag added tosrc/feature-flags/flags.ts).Based on learnings: "In the konflux-ui repository, new features should be wrapped with feature flags using the IfFeature component from '~/feature-flags/hooks' and flag definitions added to the FLAGS object in src/feature-flags/flags.ts with key, description, defaultEnabled, and status properties."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/ComponentList.tsx` around lines 33 - 178, The ComponentList UI is not gated by a feature flag: wrap the component (or its returned JSX) with IfFeature from '~/feature-flags/hooks' using the 'components-page' flag so the UI (including FeatureFlagIndicator and gettingStartedCard) only renders when enabled; import IfFeature and wrap the top-level JSX returned in ComponentList (or wrap the component export) with <IfFeature flag="components-page"> ... </IfFeature>. Also add a corresponding entry to the FLAGS object in flags.ts with key "components-page" including description, defaultEnabled and status fields so the flag exists for control.
1-29: Mixed import path styles:~/aliases and../../relative paths.The file uses
~/path aliases (lines 11–15) alongside../../relative imports (lines 16–28). Per coding guidelines, prefer absolute imports with configured path aliases (~/components,~/types, etc.) throughout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/ComponentList.tsx` around lines 1 - 29, The file mixes path alias imports (~/...) with relative imports (../../...), so update the relative imports to use the configured path aliases to match project conventions; specifically replace imports for pipelineImg, PipelineRunLabel, useLatestPushBuildPipelines, Table, useDeepCompareMemoize, AppEmptyState, FilteredEmptyState, useNamespace, ComponentKind, ButtonWithAccessTooltip, GettingStartedCard, ComponentsListHeader, ComponentsListRow and the SCSS import with their corresponding ~/... alias paths (keep the existing import names like pipelineImg, PipelineRunLabel, useLatestPushBuildPipelines, etc.), run the app build/linter to confirm no unresolved paths, and commit the updated import statements.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ComponentList/__tests__/ComponentListRow.spec.tsx`:
- Around line 31-42: The test helper renderRow is calling <ComponentsListRow>
with an obj but omits the required columns prop expected by the row renderer
type RowFunctionArgs<ComponentWithLatestBuildPipeline, PacStatesForComponents>;
update renderRow to pass a proper columns prop (e.g., an array or mock column
definitions matching the component row API) when rendering ComponentsListRow so
the TypeScript error TS2741 is resolved and the component receives the expected
columns prop.
- Around line 54-64: The test calls useNamespaceMock.mockReturnValue('test-ns')
after renderRow, but the mock is already configured in beforeEach and the
post-render call is redundant; remove the line that sets
useNamespaceMock.mockReturnValue in the test (the call on the same line as
useNamespaceMock) so the test relies on the beforeEach setup, leaving
renderRow(createMockComponent(...)) and the existing assertion that the link for
'my-comp' has href equal to COMPONENT_DETAILS_V2_PATH.createPath({
workspaceName: 'test-ns', componentName: 'my-comp' }).
---
Duplicate comments:
In `@src/components/ComponentList/ComponentList.tsx`:
- Around line 81-96: NoDataEmptyMessage renders a ButtonWithAccessTooltip that
has no onClick, so clicking does nothing; add an onClick handler by wiring the
button to the component creation flow: add an onAddComponent (or similar)
callback prop to ComponentList, implement a handler (e.g.
openCreateComponentModal or navigate to the new-component route) in the parent
or ComponentList component, and pass that handler into NoDataEmptyMessage, then
set onClick={onAddComponent} on the ButtonWithAccessTooltip inside
NoDataEmptyMessage so the "Add component" button actually triggers the create
flow.
- Around line 73-79: The name filter is currently case-sensitive in the memoized
filteredComponents logic; update the predicate inside the React.useMemo (the
filter over componentsWithLatestBuild) to compare lowercase strings by calling
toLowerCase() on both component.metadata.name (safely, e.g., via optional
chaining) and nameFilter before using indexOf, so the check becomes
case-insensitive and still respects the existing !nameFilter short-circuit.
- Around line 55-59: The hook call to useLatestPushBuildPipelines in
ComponentList.tsx is passing an empty string for applicationName which causes it
to return no pipeline runs; change the second argument from '' to the actual
application name variable (e.g., applicationName, appName, or the prop/state
that holds the current application) so the hook receives the correct application
context, and ensure that variable is defined/passed into the component (or
compute it from props/state) before calling
useLatestPushBuildPipelines(namespace, applicationName, componentNames).
In `@src/components/ComponentList/ComponentListHeader.tsx`:
- Around line 1-7: The column width classes in componentsTableColumnClasses
currently add up to 145% and cause layout overflow; update the pf-m-width-*
values for keys component, gitRepository, imageRegistry, componentVersions, and
kebab so their numeric widths total ~100% (for example adjust the percentages to
a balanced set that sums to 100), then verify the table renders without
horizontal scrolling and that wrap-column and component-list-view__actions
classes remain applied.
In `@src/components/ComponentList/ComponentListRow.tsx`:
- Around line 50-59: The raw <div> wrapping ImageUrlDisplay in
ComponentListRow.tsx (inside TableData with class
componentsTableColumnClasses.imageRegistry) lacks truncation styles and prevents
textOverflow: 'ellipsis' from working; replace the plain div with a container
that enforces single-line truncation (e.g., apply the same utility/class that
sets overflow: hidden, white-space: nowrap and text-overflow: ellipsis) or move
those styles into ImageUrlDisplay so the imageUrl (latestImage) is truncated
properly, ensuring namespace/componentName props remain unchanged.
---
Nitpick comments:
In `@src/components/ComponentList/ComponentList.tsx`:
- Around line 33-178: The ComponentList UI is not gated by a feature flag: wrap
the component (or its returned JSX) with IfFeature from '~/feature-flags/hooks'
using the 'components-page' flag so the UI (including FeatureFlagIndicator and
gettingStartedCard) only renders when enabled; import IfFeature and wrap the
top-level JSX returned in ComponentList (or wrap the component export) with
<IfFeature flag="components-page"> ... </IfFeature>. Also add a corresponding
entry to the FLAGS object in flags.ts with key "components-page" including
description, defaultEnabled and status fields so the flag exists for control.
- Around line 1-29: The file mixes path alias imports (~/...) with relative
imports (../../...), so update the relative imports to use the configured path
aliases to match project conventions; specifically replace imports for
pipelineImg, PipelineRunLabel, useLatestPushBuildPipelines, Table,
useDeepCompareMemoize, AppEmptyState, FilteredEmptyState, useNamespace,
ComponentKind, ButtonWithAccessTooltip, GettingStartedCard,
ComponentsListHeader, ComponentsListRow and the SCSS import with their
corresponding ~/... alias paths (keep the existing import names like
pipelineImg, PipelineRunLabel, useLatestPushBuildPipelines, etc.), run the app
build/linter to confirm no unresolved paths, and commit the updated import
statements.
In `@src/components/ComponentList/ComponentListRow.tsx`:
- Around line 1-14: The file mixes '~/'-aliased imports and '../../' relative
imports; update the relative imports used in this module (symbols:
PacStatesForComponents, COMPONENT_DETAILS_V2_PATH, RowFunctionArgs, TableData,
ImageUrlDisplay, useNamespace, ComponentKind, PipelineRunKind, getLastestImage,
useComponentActions, GitRepoLink, componentsTableColumnClasses) to use the
project's '~/'-alias form so all imports in ComponentListRow.tsx are consistent
with ComponentList.tsx; verify import paths resolve with the existing
tsconfig/webpack alias and run the linter/build to confirm no unresolved imports
remain.
f9428b1 to
80810ea
Compare
rrosatti
reviewed
Feb 18, 2026
| <br /> | ||
| To get started, add a component. | ||
| </EmptyStateBody> | ||
| <ButtonWithAccessTooltip variant="primary" tooltip="You don't have access to add a component"> |
Member
There was a problem hiding this comment.
can we use a simple Button here instead? also, I believe the tooltip message is not correct and the button is missing the onClick event to navigate to the "add/create component" page :)
Member
Author
There was a problem hiding this comment.
@sahil143 where should we redirect this "Add Component" button? Current Add Component is associated with application.
janaki29
force-pushed
the
component-list
branch
2 times, most recently
from
57fdb5d to
259116c
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/components/ComponentList/ComponentList.tsx (4)
118-135: Replace raw<div>wrapper with a PatternFly layout component.Line 118 wraps the
Tablein a<div>, which directly violates the project's layout guidelines. Use a PatternFly layout element instead:♻️ Proposed fix
- <div data-test="component-list"> + <React.Fragment data-test="component-list">Or, if a block container is genuinely needed for layout, use
PageSectionor a semantic PF wrapper. Thedata-testattribute can be forwarded to theTabledirectly if it accepts one.As per coding guidelines: "NEVER use raw HTML layout tags (
<div>,<section>,<header>)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/ComponentList.tsx` around lines 118 - 135, The raw <div> wrapping the Table in ComponentList should be replaced with a PatternFly layout component (e.g., PageSection or another PF container) or remove the wrapper entirely and forward the data-test attribute to the Table if it supports it; update the JSX around the Table component (the Table usage with props like virtualize, data, Header/Row, loaded, getRowProps) to use the chosen PF wrapper (or no wrapper) and ensure the data-test="component-list" is applied to the PF wrapper or the Table itself, removing the raw <div> to comply with layout guidelines.
8-18: Inconsistent import style — use~/path aliases throughout.Lines 8–18 use relative
../../imports while lines 3–7 already use~/aliases. All imports in this file should use the configured path aliases for consistency.♻️ Proposed fix
-import emptyStateImgUrl from '../../assets/Components.svg'; -import { PipelineRunLabel } from '../../consts/pipelinerun'; -import { useLatestPushBuildPipelines } from '../../hooks/useLatestPushBuildPipelines'; -import { Table, useDeepCompareMemoize } from '../../shared'; -import AppEmptyState from '../../shared/components/empty-state/AppEmptyState'; -import FilteredEmptyState from '../../shared/components/empty-state/FilteredEmptyState'; -import { useNamespace } from '../../shared/providers/Namespace/useNamespaceInfo'; -import { ComponentKind } from '../../types'; -import { ButtonWithAccessTooltip } from '../ButtonWithAccessTooltip'; -import ComponentsListHeader from './ComponentListHeader'; -import ComponentsListRow from './ComponentListRow'; +import emptyStateImgUrl from '~/assets/Components.svg'; +import { PipelineRunLabel } from '~/consts/pipelinerun'; +import { useLatestPushBuildPipelines } from '~/hooks/useLatestPushBuildPipelines'; +import { Table, useDeepCompareMemoize } from '~/shared'; +import AppEmptyState from '~/shared/components/empty-state/AppEmptyState'; +import FilteredEmptyState from '~/shared/components/empty-state/FilteredEmptyState'; +import { useNamespace } from '~/shared/providers/Namespace/useNamespaceInfo'; +import { ComponentKind } from '~/types'; +import { ButtonWithAccessTooltip } from '~/components/ButtonWithAccessTooltip'; +import ComponentsListHeader from './ComponentListHeader'; +import ComponentsListRow from './ComponentListRow';As per coding guidelines: "Use absolute imports with configured path aliases:
~/components,~/types,~/k8s,~/utils,~/models,@routes".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/ComponentList.tsx` around lines 8 - 18, Replace the relative imports in ComponentList.tsx (e.g., emptyStateImgUrl, PipelineRunLabel, useLatestPushBuildPipelines, Table, useDeepCompareMemoize, AppEmptyState, FilteredEmptyState, useNamespace, ComponentKind, ButtonWithAccessTooltip, ComponentsListHeader, ComponentsListRow) to use the project's configured path aliases (~/...) instead of '../../' so all imports follow the same absolute-alias style; update each import statement to the corresponding alias path (e.g., ~/assets, ~/consts, ~/hooks, ~/shared, ~/shared/components, ~/shared/providers, ~/types, ~/components) ensuring the symbols' import specifiers remain unchanged.
105-137: New feature should be gated withIfFeature, not just annotated withFeatureFlagIndicator.
FeatureFlagIndicatoronly renders a visual badge; it does not conditionally mount the feature. Thecomponents-pageflag is already referenced, so wrapping the return withIfFeatureis straightforward:import { IfFeature } from '~/feature-flags/hooks'; return ( <IfFeature flag="components-page"> <> ...existing JSX... </> </IfFeature> );As per coding guidelines: "Wrap new features with client-side feature flags during development."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/ComponentList.tsx` around lines 105 - 137, Wrap the ComponentList's returned JSX in the client-side feature gate by importing IfFeature and enclosing the existing fragment with <IfFeature flag="components-page">…</IfFeature>; keep or remove FeatureFlagIndicator as desired for the visual badge, but ensure the conditional mount uses IfFeature with flag "components-page" so the entire component tree (the Table, TextContent, etc.) is not mounted when the flag is off. Update imports to include IfFeature from '~/feature-flags/hooks' and apply the wrapper around the existing return fragment in the ComponentList component.
66-89: Inline component definitions inside render will cause unnecessary remounts.
NoDataEmptyMessageandEmptyMessageare declared as function components insideComponentList's render body. React uses referential equality to diff component types — a new function reference on every render means theTablecomponent will unmount and remount these subtrees on every state change (e.g., filter input), wiping their internal state and triggering layout thrash.Move both to module level (outside
ComponentList), passing required closures (e.g.,onClearFilters) as props.♻️ Proposed fix
+const NoDataEmptyMessage: React.FC = () => ( + <AppEmptyState className="component-list__empty-state" emptyStateImg={emptyStateImgUrl} title=""> + <EmptyStateBody> + A component is an image built from source code in a repository. + <br /> + To get started, add a component. + </EmptyStateBody> + <ButtonWithAccessTooltip variant="primary" tooltip="You don't have access to add a component"> + Add component + </ButtonWithAccessTooltip> + </AppEmptyState> +); + +const EmptyMessage: React.FC<{ onClearFilters: () => void }> = ({ onClearFilters }) => ( + <FilteredEmptyState + onClearFilters={onClearFilters} + data-test="components-list-view__all-filtered" + className="component-list__empty-state" + /> +); const ComponentList: React.FC = () => { ... - const NoDataEmptyMessage = () => ( ... ); - const EmptyMessage = () => ( ... ); ... + const EmptyMsg = React.useCallback(() => <EmptyMessage onClearFilters={onClearFilters} />, [onClearFilters]); ... - EmptyMsg={EmptyMessage} + EmptyMsg={EmptyMsg} NoDataEmptyMsg={NoDataEmptyMessage}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ComponentList/ComponentList.tsx` around lines 66 - 89, NoDataEmptyMessage and EmptyMessage are defined inside ComponentList's render causing new function references each render and unnecessary remounts; move both component definitions to the module level (outside ComponentList) and convert them to accept the required props (e.g., onClearFilters, emptyStateImgUrl, className, any handlers or context used inside) so they no longer close over ComponentList locals, then update where they are rendered to pass those props (for example render <NoDataEmptyMessage onClearFilters={onClearFilters} emptyStateImgUrl={emptyStateImgUrl} /> and <EmptyMessage onClearFilters={onClearFilters} />). Ensure you import/forward any UI components or utilities they use (AppEmptyState, FilteredEmptyState, ButtonWithAccessTooltip) and keep them pure to avoid remounting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/ComponentList/ComponentList.tsx`:
- Line 44: The call to useLatestPushBuildPipelines(namespace, '',
componentNames) passes an empty applicationName so the hook filters by
PipelineRunLabel.APPLICATION === '' and returns no pipelineRuns; update the hook
or call so applicationName is optional: modify useLatestPushBuildPipelines to
accept applicationName?: string and when applicationName is falsy skip the
PipelineRunLabel.APPLICATION filter (or add a new namespace-scoped hook) and
then call it from ComponentList.tsx with either undefined or a namespace-only
variant so componentsWithLatestBuild.latestBuildPipelineRun is populated for
each component.
- Around line 77-79: The "Add component" ButtonWithAccessTooltip in
ComponentList is missing an onClick handler; add an onClick that triggers the
component creation flow (e.g., use React Router's useNavigate or the existing
create handler) so the button actually navigates or opens the creation modal. In
ComponentList, import/use useNavigate (or call the existing
createComponent/openCreateModal function) and pass an onClick prop to
ButtonWithAccessTooltip that performs navigate('/components/new') or invokes the
creation function, keeping tooltip and access checks intact.
- Around line 58-64: The name filter in filteredComponents is currently
case-sensitive because the predicate uses
component.metadata.name.indexOf(nameFilter); update the filter to compare
lowercase strings by normalizing both component.metadata.name and nameFilter to
lowercase (e.g., use
component.metadata.name.toLowerCase().indexOf(nameFilter.toLowerCase()) !== -1)
so matches like "Java" will be found when the user types "java"; ensure this
change is applied inside the React.useMemo callback that computes
filteredComponents and that the dependency array remains
[componentsWithLatestBuild, nameFilter].
In `@src/components/ComponentList/ComponentListHeader.tsx`:
- Around line 1-6: componentsTableColumnClasses defines column width classes
that currently sum to 110% causing overflow; update the values in
componentsTableColumnClasses (symbol name) so the pf-m-width-* values add up to
~100 (for example redistribute widths among component, gitRepository,
imageRegistry, componentVersions) to prevent layout overflow and ensure table
cells do not compress or overflow their content.
---
Nitpick comments:
In `@src/components/ComponentList/ComponentList.tsx`:
- Around line 118-135: The raw <div> wrapping the Table in ComponentList should
be replaced with a PatternFly layout component (e.g., PageSection or another PF
container) or remove the wrapper entirely and forward the data-test attribute to
the Table if it supports it; update the JSX around the Table component (the
Table usage with props like virtualize, data, Header/Row, loaded, getRowProps)
to use the chosen PF wrapper (or no wrapper) and ensure the
data-test="component-list" is applied to the PF wrapper or the Table itself,
removing the raw <div> to comply with layout guidelines.
- Around line 8-18: Replace the relative imports in ComponentList.tsx (e.g.,
emptyStateImgUrl, PipelineRunLabel, useLatestPushBuildPipelines, Table,
useDeepCompareMemoize, AppEmptyState, FilteredEmptyState, useNamespace,
ComponentKind, ButtonWithAccessTooltip, ComponentsListHeader, ComponentsListRow)
to use the project's configured path aliases (~/...) instead of '../../' so all
imports follow the same absolute-alias style; update each import statement to
the corresponding alias path (e.g., ~/assets, ~/consts, ~/hooks, ~/shared,
~/shared/components, ~/shared/providers, ~/types, ~/components) ensuring the
symbols' import specifiers remain unchanged.
- Around line 105-137: Wrap the ComponentList's returned JSX in the client-side
feature gate by importing IfFeature and enclosing the existing fragment with
<IfFeature flag="components-page">…</IfFeature>; keep or remove
FeatureFlagIndicator as desired for the visual badge, but ensure the conditional
mount uses IfFeature with flag "components-page" so the entire component tree
(the Table, TextContent, etc.) is not mounted when the flag is off. Update
imports to include IfFeature from '~/feature-flags/hooks' and apply the wrapper
around the existing return fragment in the ComponentList component.
- Around line 66-89: NoDataEmptyMessage and EmptyMessage are defined inside
ComponentList's render causing new function references each render and
unnecessary remounts; move both component definitions to the module level
(outside ComponentList) and convert them to accept the required props (e.g.,
onClearFilters, emptyStateImgUrl, className, any handlers or context used
inside) so they no longer close over ComponentList locals, then update where
they are rendered to pass those props (for example render <NoDataEmptyMessage
onClearFilters={onClearFilters} emptyStateImgUrl={emptyStateImgUrl} /> and
<EmptyMessage onClearFilters={onClearFilters} />). Ensure you import/forward any
UI components or utilities they use (AppEmptyState, FilteredEmptyState,
ButtonWithAccessTooltip) and keep them pure to avoid remounting.
e0fe8ad to
52dc167
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
KFLUXUI-1004
Description
New Component List Page
Type of change
Screen shots / Gifs for design review
How to test or reproduce?
or just go to route ns/{ns}/components
Browser conformance:
Summary by CodeRabbit
New Features
Style
Tests
Types