Fix React 18 types#3361
Conversation
openshift-ci
bot
added
the
approved
📝 WalkthroughWalkthroughRemoved react-tagsinput and react-measure, added Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
openshift-ci
bot
added
the
size/XL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
libs/ui-lib/lib/common/components/hosts/AITable.tsx (1)
228-236: Type signature mismatch with PatternFly's OnCollapse.The
_eventparameter should be typed asReact.MouseEvent, notunknown. PatternFly'sOnCollapsetype from@patternfly/react-tablev6.4.0 expects:(event: React.MouseEvent, rowIndex: number, isOpen: boolean, rowData: IRowData, extraData: IExtraData) => void.Using
unknownactually reduces type safety compared to the properReact.MouseEventtype. While the implementation works (JavaScript allows unused parameters), the type signature should match the contract expected by PatternFly'sTablecomponent to maintain proper type checking.libs/ui-lib/lib/common/components/ui/formik/InputField.tsx (1)
62-68: Typo indata-testidattribute.There's a leading space in the test ID string which could cause test selectors to fail.
Proposed fix
<HelperTextItem variant="indeterminate" - data-testid=" input-field-description-${fieldId}-helper-text" + data-testid={`input-field-description-${fieldId}-helper-text`} >libs/ui-lib/lib/common/components/ui/StaticTextField.tsx (1)
66-77: PasschildrentoStaticFieldinFormikStaticField.
FormikStaticFieldaccepts children viaReact.PropsWithChildrenbut doesn't pass them toStaticField. When destructuring, children must be explicitly extracted and forwarded. The component is currently being used with children (e.g.,<VipStaticValue />) that are silently discarded. Update the component to match the pattern used inStaticTextField(line 83-93):const FormikStaticField: React.FC<React.PropsWithChildren<FormikStaticFieldProps>> = ({ value, children, ...props }) => { // ... return <StaticField {...props}>{children}</StaticField>; };libs/ui-lib/lib/cim/components/ClusterDeployment/hostSelection/ClusterDeploymentHostsSelectionAdvanced.tsx (1)
1-107: Remove unused duplicate component in components directory.The file at
libs/ui-lib/lib/cim/components/ClusterDeployment/components/ClusterDeploymentHostsSelectionAdvanced.tsxis an unused duplicate. The active version is atlibs/ui-lib/lib/cim/components/ClusterDeployment/hostSelection/ClusterDeploymentHostsSelectionAdvanced.tsx, which is the only one imported byScaleUpForm.tsxandClusterDeploymentHostsSelection.tsx. The components version is not exported from its directory's index file and has no usages in the codebase.
🤖 Fix all issues with AI agents
In `@libs/ui-lib/lib/common/components/ui/formik/LabelField.tsx`:
- Around line 90-99: LabelField currently renders HelperTextItem with an error
icon but never displays the actual errorMessage text; update the JSX where
HelperTextItem is used (inside FormHelperText) to pass errorMessage as its
children (e.g., <HelperTextItem ...>{errorMessage}</HelperTextItem>) so the
error text is visible to users; ensure you keep existing props like icon,
variant, id and data-testid when adding the children.
- Around line 57-63: The onEdit handler incorrectly checks label.length (always
>=1) and uses undefined for missing values causing inconsistency with onAdd;
update onEdit in LabelField.tsx so it extracts the split parts safely (e.g.,
check label.length > 1 or presence of label[1]) and assign value: '' (empty
string) when no right-hand side exists to match onAdd, then call
setLabels(newLabels, true).
- Line 29: The field value `labels` can be undefined and will cause
`labels.map()` to throw; update the code in LabelField to guard against that by
providing a default empty array for the field value or using a nullish fallback
when mapping. For example, adjust the useField destructure so the value defaults
to [] (e.g., [{ value: labels = [] }, ...]) or change the mapping site to use
(labels ?? []) before calling .map; keep existing names (`labels`, `useField`,
`setLabels`) so other logic is unchanged.
- Around line 17-20: Rename the local type "Label" to "LabelItem" to avoid
shadowing the imported PatternFly Label component: change the type declaration
"type Label = { key: string; value?: string }" to "type LabelItem = { key:
string; value?: string }" and update every usage of "Label" in this module
(e.g., the props, local variables, function signatures and array types that
currently reference Label) to use "LabelItem" instead so the PatternFly Label
import remains unshadowed.
🧹 Nitpick comments (5)
apps/assisted-ui/src/components/App.tsx (1)
12-16: Pragmatic workaround for React 18 type compatibility.The double cast through
anyis a common pattern when library types predate React 18's change whereReact.FCno longer implicitly includeschildren. This is a type-only fix with no runtime impact.Consider enhancing the comment to be more specific about the root cause for future maintainers:
📝 Suggested comment improvement
-// Add PropsWithChildren to bypass the React 18 type mismatch +// react-router-dom v5 types predate React 18 where React.FC no longer includes children. +// Wrap with PropsWithChildren to restore children prop typing. // eslint-disable-next-line const BrowserRouter18 = BrowserRouter as any as React.FC< React.PropsWithChildren<BrowserRouterProps> >;libs/ui-lib/lib/common/components/ui/CustomToolbarFilter.tsx (1)
118-123: Optional chaining improvement looks good; consider consistent type handling for portals.The refactor to use optional chaining is cleaner. However, the
as React.ReactElementcast is applied here but not to the othercreatePortalcalls at lines 133-134 and 135-137. If this cast is required to satisfy React 18's stricter typing forReactPortal, consider applying it uniformly across all portal renderings in this component for consistency.♻️ Optional: Apply consistent typing to other portals
{showToolbarItem && <ToolbarItem {...props}>{children}</ToolbarItem>} - {labelContainerRef.current && - ReactDOM.createPortal(labelGroup, labelContainerRef.current as Element)} - {expandableLabelContainerRef && - expandableLabelContainerRef.current && - ReactDOM.createPortal(labelGroup, expandableLabelContainerRef.current)} + {labelContainerRef.current + ? (ReactDOM.createPortal(labelGroup, labelContainerRef.current as Element) as React.ReactElement) + : null} + {expandableLabelContainerRef?.current + ? (ReactDOM.createPortal(labelGroup, expandableLabelContainerRef.current) as React.ReactElement) + : null}libs/ui-lib/lib/cim/components/Agent/tableUtils.tsx (1)
103-114: Consider applyingt<string>()to remaining translation calls for consistency.Within
discoveryTypeColumn, the cell content usest('ai:Unknown'),t('ai:BMC'), andt('ai:Discovery ISO')without the explicit string generic. While these may not cause type errors currently, applying the pattern consistently would improve maintainability.♻️ Suggested consistency improvement
cell: (host) => { const agent = agents.find((a) => a.metadata?.uid === host.id); - let discoveryType = t('ai:Unknown'); + let discoveryType = t<string>('ai:Unknown'); if (agent) { // eslint-disable-next-line no-prototype-builtins discoveryType = agent?.metadata?.labels?.hasOwnProperty(AGENT_BMH_NAME_LABEL_KEY) - ? t('ai:BMC') - : t('ai:Discovery ISO'); + ? t<string>('ai:BMC') + : t<string>('ai:Discovery ISO'); } else { const bmh = bareMetalHosts.find((bmh) => bmh.metadata?.uid === host.id); if (bmh) { - discoveryType = t('ai:BMC'); + discoveryType = t<string>('ai:BMC'); } }libs/ui-lib/lib/common/components/hosts/tableUtils.tsx (1)
325-383: Consider adding i18n for hardcoded column headers.The
ipv4Column,ipv6Column, andmacAddressColumnheaders use hardcoded English strings ('IPv4 address','IPv6 address','MAC address') instead of translation keys. For consistency with other columns in this file, these should use thet<string>()pattern.♻️ Suggested i18n improvement
-export const ipv4Column = (cluster: Cluster): TableRow<Host> => ({ +export const ipv4Column = (cluster: Cluster, t: TFunction): TableRow<Host> => ({ header: { - title: 'IPv4 address', + title: t<string>('ai:IPv4 address'), sort: true, }, // ... cell implementation }); -export const ipv6Column = (cluster: Cluster): TableRow<Host> => ({ +export const ipv6Column = (cluster: Cluster, t: TFunction): TableRow<Host> => ({ header: { - title: 'IPv6 address', + title: t<string>('ai:IPv6 address'), sort: true, }, // ... cell implementation }); -export const macAddressColumn = (cluster: Cluster): TableRow<Host> => ({ +export const macAddressColumn = (cluster: Cluster, t: TFunction): TableRow<Host> => ({ header: { - title: 'MAC address', + title: t<string>('ai:MAC address'), sort: true, }, // ... cell implementation });libs/ui-lib/lib/cim/components/YamlPreview/YamlPreview.tsx (1)
56-60: Consider initial height measurement timing.The
useResizeObservercallback fires when the container resizes, but there's no initial measurement. The editor won't render until a resize event occurs. This may cause a brief delay before the editor appears.Optional: Add initial height measurement
const editorRef = React.useRef<HTMLDivElement>(null); const [editorHeight, setEditorHeight] = React.useState<number>(); + + React.useLayoutEffect(() => { + if (editorRef.current) { + setEditorHeight(editorRef.current.getBoundingClientRect().height); + } + }, []); + useResizeObserver(editorRef, (entry) => { setEditorHeight(entry.contentRect.height); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (42)
apps/assisted-disconnected-ui/package.jsonapps/assisted-ui/package.jsonapps/assisted-ui/src/components/App.tsxlibs/ui-lib/lib/cim/components/Agent/tableUtils.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/components/ClusterDeploymentHostsSelectionAdvanced.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/customManifests/ConfigMapForm.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/hostDiscovery/ClusterDeploymentHostsDiscovery.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/hostSelection/ClusterDeploymentHostsSelectionAdvanced.tsxlibs/ui-lib/lib/cim/components/InfraEnv/InfraEnvFormPage.tsxlibs/ui-lib/lib/cim/components/YamlPreview/YamlPreview.tsxlibs/ui-lib/lib/cim/components/modals/EditSSHKeyModal.tsxlibs/ui-lib/lib/cim/components/modals/MassApproveAgentModal.tsxlibs/ui-lib/lib/cim/components/modals/MassDeleteAgentModal.tsxlibs/ui-lib/lib/common/components/AddHosts/AddHostsContext.tsxlibs/ui-lib/lib/common/components/AlertsContextProvider.tsxlibs/ui-lib/lib/common/components/clusterWizard/ClusterWizardStepHeader.tsxlibs/ui-lib/lib/common/components/hosts/AITable.tsxlibs/ui-lib/lib/common/components/hosts/HostPropertyValidationPopover.tsxlibs/ui-lib/lib/common/components/hosts/HostStatus.tsxlibs/ui-lib/lib/common/components/hosts/HostsTableDetailContext.tsxlibs/ui-lib/lib/common/components/hosts/TableToolbar.tsxlibs/ui-lib/lib/common/components/hosts/tableUtils.tsxlibs/ui-lib/lib/common/components/storage/DisksTable.tsxlibs/ui-lib/lib/common/components/ui/CustomToolbarFilter.tsxlibs/ui-lib/lib/common/components/ui/GridGap.tsxlibs/ui-lib/lib/common/components/ui/StaticTextField.tsxlibs/ui-lib/lib/common/components/ui/Toolbar/ToolbarSecondaryGroup.tsxlibs/ui-lib/lib/common/components/ui/formik/CertificatesUploadField.tsxlibs/ui-lib/lib/common/components/ui/formik/InputField.tsxlibs/ui-lib/lib/common/components/ui/formik/LabelField.tsxlibs/ui-lib/lib/common/components/ui/formik/NumberInputField.tsxlibs/ui-lib/lib/common/components/ui/formik/SelectFieldWithSearch.tsxlibs/ui-lib/lib/common/components/ui/formik/TextAreaField.tsxlibs/ui-lib/lib/common/components/ui/formik/UploadField.tsxlibs/ui-lib/lib/common/features/featureGate.tsxlibs/ui-lib/lib/ocm/components/SentryErrorMonitorContextProvider.tsxlibs/ui-lib/lib/ocm/components/clusters/ClusterToolbar.tsxlibs/ui-lib/lib/ocm/components/hosts/ModalDialogsContext.tsxlibs/ui-lib/lib/ocm/components/ui/AssistedUILibVersion.tsxlibs/ui-lib/lib/ocm/store/slices/clusters/selectors.tsxlibs/ui-lib/package.jsonpackage.json
💤 Files with no reviewable changes (3)
- apps/assisted-ui/package.json
- package.json
- apps/assisted-disconnected-ui/package.json
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-12-17T09:08:18.279Z
Learnt from: jgyselov
Repo: openshift-assisted/assisted-installer-ui PR: 3319
File: libs/ui-lib/lib/cim/components/helpers/toAssisted.ts:199-201
Timestamp: 2025-12-17T09:08:18.279Z
Learning: In libs/ui-lib/lib/cim/components/helpers/toAssisted.ts, the platformType field on agentClusterInstall.spec is guaranteed to always contain a valid PlatformType value (when lowercased), making the type assertion safe without additional runtime validation.
Applied to files:
libs/ui-lib/lib/ocm/components/ui/AssistedUILibVersion.tsxlibs/ui-lib/lib/ocm/store/slices/clusters/selectors.tsxlibs/ui-lib/lib/cim/components/modals/MassDeleteAgentModal.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/customManifests/ConfigMapForm.tsxlibs/ui-lib/lib/cim/components/Agent/tableUtils.tsx
📚 Learning: 2025-10-21T04:40:36.292Z
Learnt from: linoyaslan
Repo: openshift-assisted/assisted-installer-ui PR: 3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx:55-63
Timestamp: 2025-10-21T04:40:36.292Z
Learning: In libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx, the network reordering logic in the useEffect (swapping clusterNetworks and serviceNetworks based on the primary machine network's IP family) is for UI consistency only. Validation of empty or invalid CIDRs is handled separately by validation schemas, not by the reordering logic.
Applied to files:
libs/ui-lib/lib/cim/components/ClusterDeployment/hostDiscovery/ClusterDeploymentHostsDiscovery.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/hostSelection/ClusterDeploymentHostsSelectionAdvanced.tsxlibs/ui-lib/lib/ocm/store/slices/clusters/selectors.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/components/ClusterDeploymentHostsSelectionAdvanced.tsx
📚 Learning: 2025-12-17T09:16:41.439Z
Learnt from: jgyselov
Repo: openshift-assisted/assisted-installer-ui PR: 3319
File: libs/ui-lib/lib/common/components/hosts/utils.ts:208-210
Timestamp: 2025-12-17T09:16:41.439Z
Learning: In libs/ui-lib/lib/common/components/hosts/utils.ts, the nodeLabels field on Host objects is guaranteed to be valid JSON because it's always constructed using JSON.stringify in libs/ui-lib/lib/cim/components/helpers/toAssisted.ts, making direct JSON.parse safe without additional error handling.
Applied to files:
libs/ui-lib/lib/common/components/hosts/tableUtils.tsxlibs/ui-lib/lib/common/components/hosts/HostPropertyValidationPopover.tsx
📚 Learning: 2025-10-20T08:57:04.935Z
Learnt from: linoyaslan
Repo: openshift-assisted/assisted-installer-ui PR: 3190
File: libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx:43-45
Timestamp: 2025-10-20T08:57:04.935Z
Learning: In the file `libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx`, the `getStackTypeLabel` function returns 'Dual-stack' or 'IPv4' because the system currently only supports IPv4 single-stack clusters. IPv6-only single-stack clusters are not supported.
Applied to files:
libs/ui-lib/lib/common/components/hosts/tableUtils.tsx
📚 Learning: 2026-01-07T15:19:02.310Z
Learnt from: rawagner
Repo: openshift-assisted/assisted-installer-ui PR: 3350
File: apps/assisted-ui/package.json:26-27
Timestamp: 2026-01-07T15:19:02.310Z
Learning: The react-router-dom-v5-compat package is designed to work alongside react-router-dom v4 or v5, not replace it. It declares peer dependencies on react-router-dom "4 || 5" and allows incremental migration by providing React Router v6 APIs while the application continues to run on v5. Having both react-router-dom5.x and react-router-dom-v5-compat installed together is the correct and intended configuration.
Applied to files:
apps/assisted-ui/src/components/App.tsxlibs/ui-lib/package.json
📚 Learning: 2025-05-22T09:12:50.632Z
Learnt from: rawagner
Repo: openshift-assisted/assisted-installer-ui PR: 2942
File: libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentManifestsStep.tsx:93-97
Timestamp: 2025-05-22T09:12:50.632Z
Learning: When working with Formik's FieldArray, use the provided array helpers (like `remove()`, `push()`, etc.) instead of directly mutating the values array to ensure proper state tracking and re-rendering.
Applied to files:
libs/ui-lib/lib/common/components/ui/StaticTextField.tsxlibs/ui-lib/lib/common/components/ui/formik/LabelField.tsx
📚 Learning: 2025-05-22T07:31:16.062Z
Learnt from: rawagner
Repo: openshift-assisted/assisted-installer-ui PR: 2826
File: libs/ui-lib/lib/cim/components/YamlEditor/YamlEditor.tsx:50-61
Timestamp: 2025-05-22T07:31:16.062Z
Learning: The YamlEditor component in libs/ui-lib/lib/cim/components/YamlEditor/YamlEditor.tsx already handles YAML validation through the submitYamls function in utils.ts, which validates YAML syntax and throws user-friendly error messages if parsing fails.
Applied to files:
libs/ui-lib/lib/cim/components/YamlPreview/YamlPreview.tsx
🧬 Code graph analysis (10)
libs/ui-lib/lib/common/components/ui/formik/NumberInputField.tsx (1)
libs/ui-lib/lib/common/components/ui/formik/types.ts (1)
NumberInputFieldProps(76-86)
libs/ui-lib/lib/cim/components/modals/MassApproveAgentModal.tsx (3)
libs/ui-lib/lib/common/components/hosts/tableUtils.tsx (1)
hostnameColumn(59-92)libs/ui-lib/lib/cim/types/k8s/agent.ts (1)
AgentK8sResource(32-74)libs/ui-lib/lib/common/components/hosts/AITable.tsx (1)
TableRow(24-28)
libs/ui-lib/lib/common/components/hosts/HostStatus.tsx (1)
libs/ui-lib/lib/common/components/hosts/types.ts (2)
HostStatus(81-83)HostStatusProps(85-95)
libs/ui-lib/lib/common/components/ui/formik/TextAreaField.tsx (1)
libs/ui-lib/lib/common/components/ui/formik/types.ts (1)
TextAreaFieldProps(88-95)
apps/assisted-ui/src/components/App.tsx (2)
apps/assisted-ui/src/components/Header.tsx (1)
Header(6-24)libs/ui-lib/lib/ocm/components/Routes.tsx (1)
UILibRoutes(25-61)
libs/ui-lib/lib/common/components/ui/formik/InputField.tsx (1)
libs/ui-lib/lib/common/components/ui/formik/types.ts (1)
InputFieldProps(66-74)
libs/ui-lib/lib/common/components/ui/formik/UploadField.tsx (1)
libs/ui-lib/lib/common/components/ui/formik/types.ts (1)
UploadFieldProps(97-106)
libs/ui-lib/lib/ocm/store/slices/clusters/selectors.tsx (1)
libs/ui-lib/lib/common/components/ui/table/utils.ts (1)
HumanizedSortable(7-11)
libs/ui-lib/lib/common/components/ui/formik/CertificatesUploadField.tsx (1)
libs/ui-lib/lib/common/components/ui/formik/types.ts (1)
UploadFieldProps(97-106)
libs/ui-lib/lib/cim/components/modals/EditSSHKeyModal.tsx (1)
libs/ui-lib/lib/cim/components/modals/types.ts (1)
EditSSHKeyFormikValues(52-54)
🪛 Biome (2.1.2)
libs/ui-lib/lib/common/components/ui/formik/LabelField.tsx
[error] 17-17: Shouldn't redeclare 'Label'. Consider to delete it or rename it.
'Label' is defined here:
(lint/suspicious/noRedeclare)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: circular-deps
- GitHub Check: tests
- GitHub Check: translation-files
- GitHub Check: format
🔇 Additional comments (69)
libs/ui-lib/lib/common/features/featureGate.tsx (1)
37-41: LGTM!Correctly wraps props with
React.PropsWithChildrento explicitly declarechildrensupport, which is required for React 18 types sinceReact.FCno longer implicitly includeschildrenin its props.libs/ui-lib/lib/cim/components/modals/EditSSHKeyModal.tsx (2)
72-72: Correct type alignment for Formik render props.The render-prop generic now correctly uses
EditSSHKeyFormikValues(the actual form values type) instead ofEditSSHKeyModalProps(the component props). This aligns with theFormik<EditSSHKeyFormikValues>generic on line 56.
90-90: Proper handling of async event handler.Using
() => void submitForm()is the correct pattern for React 18 types. Thevoidoperator explicitly discards the promise returned bysubmitForm, satisfying TypeScript's stricter return type checking foronClickhandlers (which expectvoid, notPromise<void>).libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvFormPage.tsx (2)
133-140: LGTM!Correct use of
React.PropsWithChildrento explicitly type children for React 18+, whereReact.FCno longer implicitly includeschildren. The component properly destructures and renders children.
289-297: LGTM!Appropriate use of
React.PropsWithChildrenwrapper. The component correctly destructures children and passes them to the nestedInfraEnvFormcomponent.libs/ui-lib/lib/common/components/hosts/HostStatus.tsx (2)
186-188: LGTM!Correct React 18 typing fix. The component uses
props.children(line 204), so wrapping withReact.PropsWithChildrenproperly declares the implicit children prop that was removed fromReact.FCin React 18.
231-243: LGTM!Proper React 18 typing update. The destructured
childrenprop (line 238) and its usage (lines 282, 287) require explicit typing viaReact.PropsWithChildren. This aligns with the broader codebase standardization mentioned in the PR summary.libs/ui-lib/lib/cim/components/ClusterDeployment/customManifests/ConfigMapForm.tsx (1)
220-228: LGTM!The
!!isErrorcoercion ensures a boolean value is used for conditional rendering, which aligns with React 18's stricter TypeScript types and is consistent with the existing pattern on line 230 (!!alerts.length).libs/ui-lib/lib/cim/components/modals/MassApproveAgentModal.tsx (3)
21-24: LGTM! Correct React 18 typing for children prop.The use of
React.PropsWithChildren<ApproveTableRowProps>is the proper approach for React 18, wherechildrenis no longer implicitly included inReact.FC. The explicit destructuring and rendering ofchildrenis correct.
29-29: LGTM! Consistent with codebase patterns.The explicit
t<string>(...)generic matches the pattern used intableUtils.tsxfor header titles, ensuring proper type narrowing for the translation return value.
54-54: LGTM!Consistent application of the
t<string>(...)pattern for header titles.apps/assisted-ui/src/components/App.tsx (2)
2-2: LGTM!Clean import addition to support the type wrapper below.
18-27: LGTM!The component structure remains intact with the wrapper cleanly applied. The routing hierarchy (BrowserRouter18 → CompatRouter → Page → UILibRoutes) correctly maintains the v5-compat migration pattern per the learned context.
libs/ui-lib/lib/common/components/storage/DisksTable.tsx (1)
42-44: LGTM!The explicit
t<string>()generic ensures type safety for thebodyContentprop, which is consistent with the PR-wide pattern of tightening i18n translation typings.libs/ui-lib/lib/cim/components/modals/MassDeleteAgentModal.tsx (3)
33-33: LGTM!Correct application of
t<string>()for the header title type safety.
66-66: LGTM!Consistent with the hostname column pattern above.
88-103: LGTM!All Popover content strings and the interpolated translation on line 98 correctly use the
t<string>()pattern. The interpolation syntaxt<string>('ai:Go to cluster {{clusterName}}', { clusterName })is properly preserved.libs/ui-lib/lib/cim/components/Agent/tableUtils.tsx (5)
48-48: LGTM!The
t<string>()pattern is correctly applied to the header title.
95-95: LGTM!Consistent typing for the discovery type header.
149-149: LGTM!Consistent typing for the status header.
197-197: LGTM!Consistent typing for the cluster header.
237-237: LGTM!Consistent typing for the infrastructure env header.
libs/ui-lib/lib/common/components/hosts/tableUtils.tsx (11)
67-67: LGTM!Correct application of
t<string>()for the hostname header.
103-103: LGTM!Consistent typing for the role header.
135-135: LGTM!Consistent typing for the status header.
172-172: LGTM!Consistent typing for the discovered-on header.
191-191: LGTM!Consistent typing for the CPU architecture header.
209-209: LGTM!Consistent typing for the CPU cores header.
236-236: LGTM!Consistent typing for the memory header.
261-261: LGTM!Consistent typing for the total storage header.
286-286: LGTM!Consistent typing for the GPUs header.
308-308: LGTM!Consistent typing for the active NIC header.
387-387: LGTM!Consistent typing for the labels header.
libs/ui-lib/lib/common/components/ui/formik/LabelField.tsx (2)
109-158: LGTM!The
EditableLabelControlfollows common inline editing patterns with proper keyboard handling (Enter to confirm, Escape to discard) and accessibility support via translated aria-labels.
65-89: Good implementation of the LabelGroup with editable labels.The rendering logic correctly handles the edit/delete flows and the key generation is appropriate for this use case.
libs/ui-lib/lib/common/components/ui/formik/SelectFieldWithSearch.tsx (1)
166-184: Type refinement is appropriate.Typing
valasunknownis a reasonable choice here since:
- The value originates from PatternFly's
Selectcomponent which passesselectionof varying types- The
useField<unknown | unknown[]>on line 51 already acknowledges the loosely-typed nature of the field value- The callback body handles
valdefensively with truthy checks before useThe implementation correctly handles the
unknowntype throughout the callback logic.libs/ui-lib/lib/common/components/AddHosts/AddHostsContext.tsx (1)
13-22: LGTM!The update to
React.PropsWithChildren<AddHostsContextType>is the correct approach for React 18 compatibility, whereReact.FCno longer implicitly includeschildrenin its props type. The component correctly useschildrenand renders it within the provider.libs/ui-lib/lib/ocm/store/slices/clusters/selectors.tsx (1)
62-71: LGTM!The
as HumanizedSortabletype assertions on thebaseDnsDomainandversionTextcells are consistent with the other cells in this function and correctly match theHumanizedSortableinterface structure (title,sortableValue,props).libs/ui-lib/lib/common/components/hosts/TableToolbar.tsx (1)
38-46: LGTM!The
React.PropsWithChildrenwrapper correctly addresses React 18's removal of implicitchildrenfromReact.FC. The component destructures and renderschildrenappropriately.libs/ui-lib/lib/ocm/components/clusters/ClusterToolbar.tsx (1)
9-12: LGTM!Correct React 18 typing update. The component renders
childrenwithinToolbarContent, soReact.PropsWithChildrenis necessary.libs/ui-lib/lib/common/components/ui/formik/UploadField.tsx (1)
18-30: LGTM!Correct React 18 typing update. The component renders
childrenwithin theFormGroup(line 50), soReact.PropsWithChildrenis required to explicitly type thechildrenprop.libs/ui-lib/lib/ocm/components/ui/AssistedUILibVersion.tsx (1)
4-4: LGTM!Correct fix for React 18 type compatibility. Since
React.FCno longer implicitly includeschildrenin React 18's type definitions, wrapping withReact.PropsWithChildrenis the appropriate pattern.libs/ui-lib/lib/common/components/ui/GridGap.tsx (1)
7-9: LGTM!Correct React 18 type fix. The component properly uses
React.PropsWithChildrento explicitly type the children prop.libs/ui-lib/lib/ocm/components/SentryErrorMonitorContextProvider.tsx (1)
11-13: LGTM!Correct React 18 type fix for the context provider. Explicitly typing
childrenwithReact.PropsWithChildrenis the right approach for provider components in React 18.libs/ui-lib/lib/ocm/components/hosts/ModalDialogsContext.tsx (1)
107-107: LGTM!Correct React 18 type fix for the modal dialogs context provider. The explicit
React.PropsWithChildrentyping is appropriate since the component renderschildrenwithin the context provider.libs/ui-lib/lib/common/components/ui/formik/NumberInputField.tsx (1)
18-18: LGTM!Correct React 18 type fix. Using
React.PropsWithChildren<NumberInputFieldProps>properly extends the existing props interface with explicit children typing, which is necessary since the component renders children in line 84.libs/ui-lib/lib/common/components/AlertsContextProvider.tsx (1)
21-21: LGTM!Correct React 18 typing fix. Since
React.FCno longer implicitly includeschildren, usingReact.PropsWithChildrenis the appropriate pattern for components that render children.libs/ui-lib/lib/common/components/ui/formik/TextAreaField.tsx (1)
14-14: LGTM!Proper React 18 typing. The component renders
children(line 48) inside theFormGroup, so wrappingTextAreaFieldPropswithReact.PropsWithChildrenis the correct approach.libs/ui-lib/lib/common/components/clusterWizard/ClusterWizardStepHeader.tsx (1)
4-4: LGTM!Correct typing for React 18. The component renders children within the
Contentelement, requiring explicitchildrenprop typing.libs/ui-lib/lib/common/components/ui/formik/CertificatesUploadField.tsx (1)
17-17: LGTM!Correct React 18 typing. The component renders
children(line 93) inside theFormGroup, so wrappingUploadFieldPropswithReact.PropsWithChildrenis appropriate.libs/ui-lib/lib/common/components/ui/Toolbar/ToolbarSecondaryGroup.tsx (1)
4-4: LGTM!Correct typing for React 18. The component renders children within
ToolbarGroup, requiring explicitchildrenprop typing.libs/ui-lib/lib/common/components/ui/formik/InputField.tsx (1)
17-21: LGTM! Correct React 18 type migration.The
React.PropsWithChildrenwrapper correctly makeschildrenexplicit in the component's type signature, which is required in React 18 sinceReact.FCno longer implicitly includes children.libs/ui-lib/lib/common/components/hosts/HostsTableDetailContext.tsx (1)
15-29: LGTM! Correct React 18 type migration for context provider.The
React.PropsWithChildrenwrapper is correctly applied to the provider component.Minor observation: The type assertion
as HostsTableDetailContextValueon line 25 appears redundant sincecontextis already typed correctly, but this is an existing pattern and not introduced by this change.libs/ui-lib/lib/common/components/hosts/HostPropertyValidationPopover.tsx (2)
18-22: LGTM! Correct React 18 type migration.The
React.PropsWithChildrenwrapper is correctly applied, andchildrenis properly destructured and used within theButtoncomponent.
54-64: LGTM! Consistent type migration pattern.The
React.PropsWithChildrenwrapper follows the same correct pattern asValidationPopoverabove, withchildrenproperly destructured and rendered in multiple conditional branches.libs/ui-lib/lib/common/components/ui/StaticTextField.tsx (2)
22-30: LGTM! Correct React 18 type migration for StaticField.The
React.PropsWithChildrenwrapper is correctly applied, withchildrenproperly destructured and rendered.
83-95: LGTM! Correct React 18 type migration for StaticTextField.The
React.PropsWithChildrenwrapper is correctly applied, withchildrenexplicitly destructured and passed to theContentcomponent.libs/ui-lib/lib/cim/components/YamlPreview/YamlPreview.tsx (4)
2-4: Good migration from react-measure to useResizeObserver.The imports correctly bring in the Monaco editor types and the resize observer hook, replacing the deprecated
react-measuredependency as mentioned in the PR objectives.
17-33: LGTM! Well-typed Monaco theme definition.The explicit
editor.IStandaloneThemeDatatyping ensures type safety for the custom theme configuration.
88-100: Conditional rendering guards against zero-height editor.The
!!editorHeightcheck prevents rendering the Monaco editor before a valid height is determined, avoiding layout issues. This is the correct approach for resize-observer-based sizing.
41-46: LGTM! Correct React 18 type migration.The
React.PropsWithChildrenwrapper is correctly applied, withchildrenproperly destructured and rendered in the readonly-editor-bar div.libs/ui-lib/package.json (2)
13-13: LGTM! Good choice for replacing react-measure.
@react-hook/resize-observeris a lightweight, well-maintained library that provides a cleaner API using hooks, which aligns well with modern React patterns.
45-47: Type versions are aligned correctly.Pinning
@types/react-domto18.2.15matches the pinned@types/reactat18.2.37, both corresponding to React 18.2.x types. This avoids potential type mismatches.libs/ui-lib/lib/cim/components/ClusterDeployment/components/ClusterDeploymentHostsSelectionAdvanced.tsx (2)
61-65: Resize observer implementation looks correct.Same pattern as the other file -
tableWidthstarts asundefinedand gets populated after the observer fires. The implementation is functionally correct.
93-102: Clean refactor from Measure to ResizeObserver.The wrapper
divwithrefand passingtableWidthto the table component is the correct approach for the resize observer pattern.libs/ui-lib/lib/cim/components/ClusterDeployment/hostDiscovery/ClusterDeploymentHostsDiscovery.tsx (3)
3-3: LGTM!Import is consistent with the other refactored components.
71-75: Consistent resize observer pattern.Good to see the same pattern applied consistently across components. The hook correctly observes the container div and updates
tableWidthstate.
109-124: Clean migration from Measure to ResizeObserver.The table is now wrapped in a div with a ref, and the observed width is passed down. This maintains the same width-driven behavior with a more modern and lightweight approach.
libs/ui-lib/lib/cim/components/ClusterDeployment/hostSelection/ClusterDeploymentHostsSelectionAdvanced.tsx (1)
61-65: No action needed —AgentsSelectionTablealready handlesundefinedwidth gracefully.The component defines
width?: number(line 43) and uses the patternconst addAll = width && width > 700(line 101) to safely check if width exists and exceeds 700. This defensive check prevents errors, and the conditional rendering logic (lines 113, 121) gracefully degrades by omitting certain columns when width is undefined. The table renders correctly in all cases.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/ui-lib/lib/common/components/ui/formik/InputField.tsx (1)
64-64: Pre-existing typo in test ID.The
data-testidhas a leading space:" input-field-description-...". This could cause test selector issues.Proposed fix
- data-testid=" input-field-description-${fieldId}-helper-text" + data-testid={`input-field-description-${fieldId}-helper-text`}Note: The template literal also isn't being interpolated (missing backticks), so
${fieldId}appears literally in the output.
🤖 Fix all issues with AI agents
In `@libs/locales/lib/en/translation.json`:
- Line 974: The translation key "New label" is missing the required ai:
namespace; change the JSON key from "New label" to "ai:New label" (keeping the
value "New label") and ensure no duplicate "ai:New label" key already exists,
updating any code references if they expect the non-namespaced key so everything
remains consistent with other keys like "ai:Add label" and "ai:Add more".
- Line 2: Remove the duplicate un-prefixed JSON key "Add label" from the
translations so only the existing namespaced "ai:Add label": "Add label"
remains; locate the un-prefixed key "Add label" in the translations JSON and
delete that entry to avoid redundancy and key collisions.
In `@libs/ui-lib/lib/common/components/ui/formik/LabelField.tsx`:
- Around line 135-143: There’s a race where onBlur calls onConfirmAdd while
onKeyDown handling Escape calls onDiscardAdd; fix by adding a ref flag (e.g.,
isDiscardingRef) that onKeyDown sets when Escape is pressed and onDiscardAdd
runs, then check that ref in the onBlur handler before invoking onConfirmAdd
(and clear the ref after discard); update handlers referenced (onKeyDown,
onDiscardAdd, onBlur, onConfirmAdd) so blur is suppressed when isDiscardingRef
is true to prevent the confirm from firing.
♻️ Duplicate comments (3)
libs/ui-lib/lib/common/components/ui/formik/LabelField.tsx (3)
17-20: Rename the localLabeltype to avoid shadowing the PatternFly import.The local type shadows the imported
Labelcomponent from PatternFly (line 8). Rename toLabelItemor similar to avoid confusion and tooling issues.
29-29: Guard against undefinedlabelsto prevent runtime errors.If the form field isn't initialized,
labels.map()at line 71 will throw. Provide a fallback:- const [{ value: labels }, { touched, error }, { setValue: setLabels }] = useField<Label[]>(name); + const [{ value }, { touched, error }, { setValue: setLabels }] = useField<Label[]>(name); + const labels = value ?? [];
57-63: Fix the condition and inconsistent value handling inonEdit.Two issues:
label.length ? label[1] : undefined— aftersplit('='), the array always has length ≥ 1, so this condition is always truthy. Should belabel.length > 1.- Inconsistency:
onAdd(line 47) setsvalue: split[1]orvalue: '', whileonEditsetsvalue: undefined. This can cause subtle comparison bugs.const onEdit = (index: number, nextText: string) => { const label = nextText.split('='); const newLabels = [...labels]; - newLabels.splice(index, 1, { key: label[0], value: label.length ? label[1] : undefined }); + newLabels.splice(index, 1, { key: label[0], value: label.length > 1 ? label[1] : '' }); setLabels(newLabels, true); };
🧹 Nitpick comments (3)
libs/ui-lib/lib/cim/components/YamlPreview/YamlPreview.tsx (1)
88-100: Consider edge case when container height is zero.The
!!editorHeightcheck will prevent rendering both wheneditorHeightisundefined(initial state) and when it's0. If the container ever has a legitimate height of 0 (e.g., during CSS transitions or when collapsed), the editor won't render.If this is intentional behavior, the current implementation is fine. Otherwise, consider explicitly checking for
undefined:♻️ Optional: Explicit undefined check
- {!!editorHeight && ( + {editorHeight !== undefined && (apps/assisted-ui/src/components/App.tsx (1)
12-16: Reasonable workaround for React 18 type compatibility.The double cast pattern is a pragmatic solution for the React 18
childrentype change when using react-router-dom v5 types. This approach maintains functionality during the incremental migration to v6.Minor nit: consider making the eslint-disable more specific:
-// eslint-disable-next-line +// eslint-disable-next-line `@typescript-eslint/no-explicit-any`libs/ui-lib/lib/ocm/store/slices/clusters/selectors.tsx (1)
62-71: Type casts align with existing pattern - LGTM with minor suggestion.The explicit
as HumanizedSortablecasts make these cells consistent with the others in the array.One small note: on line 65,
sortableValue: baseDnsDomaincould beundefined(per line 34), butHumanizedSortable.sortableValueexpectsnumber | string. The cast works around this, but for better type safety, consider defaulting to an empty string:Optional improvement
{ title: baseDnsDomain || DASH, props: { 'data-testid': `cluster-base-domain-${name}` }, - sortableValue: baseDnsDomain, + sortableValue: baseDnsDomain || '', } as HumanizedSortable,
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (43)
apps/assisted-disconnected-ui/package.jsonapps/assisted-ui/package.jsonapps/assisted-ui/src/components/App.tsxlibs/locales/lib/en/translation.jsonlibs/ui-lib/lib/cim/components/Agent/tableUtils.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/components/ClusterDeploymentHostsSelectionAdvanced.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/customManifests/ConfigMapForm.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/hostDiscovery/ClusterDeploymentHostsDiscovery.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/hostSelection/ClusterDeploymentHostsSelectionAdvanced.tsxlibs/ui-lib/lib/cim/components/InfraEnv/InfraEnvFormPage.tsxlibs/ui-lib/lib/cim/components/YamlPreview/YamlPreview.tsxlibs/ui-lib/lib/cim/components/modals/EditSSHKeyModal.tsxlibs/ui-lib/lib/cim/components/modals/MassApproveAgentModal.tsxlibs/ui-lib/lib/cim/components/modals/MassDeleteAgentModal.tsxlibs/ui-lib/lib/common/components/AddHosts/AddHostsContext.tsxlibs/ui-lib/lib/common/components/AlertsContextProvider.tsxlibs/ui-lib/lib/common/components/clusterWizard/ClusterWizardStepHeader.tsxlibs/ui-lib/lib/common/components/hosts/AITable.tsxlibs/ui-lib/lib/common/components/hosts/HostPropertyValidationPopover.tsxlibs/ui-lib/lib/common/components/hosts/HostStatus.tsxlibs/ui-lib/lib/common/components/hosts/HostsTableDetailContext.tsxlibs/ui-lib/lib/common/components/hosts/TableToolbar.tsxlibs/ui-lib/lib/common/components/hosts/tableUtils.tsxlibs/ui-lib/lib/common/components/storage/DisksTable.tsxlibs/ui-lib/lib/common/components/ui/CustomToolbarFilter.tsxlibs/ui-lib/lib/common/components/ui/GridGap.tsxlibs/ui-lib/lib/common/components/ui/StaticTextField.tsxlibs/ui-lib/lib/common/components/ui/Toolbar/ToolbarSecondaryGroup.tsxlibs/ui-lib/lib/common/components/ui/formik/CertificatesUploadField.tsxlibs/ui-lib/lib/common/components/ui/formik/InputField.tsxlibs/ui-lib/lib/common/components/ui/formik/LabelField.tsxlibs/ui-lib/lib/common/components/ui/formik/NumberInputField.tsxlibs/ui-lib/lib/common/components/ui/formik/SelectFieldWithSearch.tsxlibs/ui-lib/lib/common/components/ui/formik/TextAreaField.tsxlibs/ui-lib/lib/common/components/ui/formik/UploadField.tsxlibs/ui-lib/lib/common/features/featureGate.tsxlibs/ui-lib/lib/ocm/components/SentryErrorMonitorContextProvider.tsxlibs/ui-lib/lib/ocm/components/clusters/ClusterToolbar.tsxlibs/ui-lib/lib/ocm/components/hosts/ModalDialogsContext.tsxlibs/ui-lib/lib/ocm/components/ui/AssistedUILibVersion.tsxlibs/ui-lib/lib/ocm/store/slices/clusters/selectors.tsxlibs/ui-lib/package.jsonpackage.json
💤 Files with no reviewable changes (3)
- apps/assisted-ui/package.json
- apps/assisted-disconnected-ui/package.json
- package.json
🚧 Files skipped from review as they are similar to previous changes (20)
- libs/ui-lib/lib/common/components/clusterWizard/ClusterWizardStepHeader.tsx
- libs/ui-lib/lib/ocm/components/clusters/ClusterToolbar.tsx
- libs/ui-lib/lib/cim/components/ClusterDeployment/customManifests/ConfigMapForm.tsx
- libs/ui-lib/lib/cim/components/modals/EditSSHKeyModal.tsx
- libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvFormPage.tsx
- libs/ui-lib/lib/common/components/ui/formik/NumberInputField.tsx
- libs/ui-lib/lib/common/components/hosts/TableToolbar.tsx
- libs/ui-lib/lib/cim/components/Agent/tableUtils.tsx
- libs/ui-lib/lib/common/components/ui/formik/SelectFieldWithSearch.tsx
- libs/ui-lib/package.json
- libs/ui-lib/lib/ocm/components/hosts/ModalDialogsContext.tsx
- libs/ui-lib/lib/common/components/ui/formik/UploadField.tsx
- libs/ui-lib/lib/common/components/hosts/tableUtils.tsx
- libs/ui-lib/lib/cim/components/ClusterDeployment/hostSelection/ClusterDeploymentHostsSelectionAdvanced.tsx
- libs/ui-lib/lib/ocm/components/SentryErrorMonitorContextProvider.tsx
- libs/ui-lib/lib/common/components/storage/DisksTable.tsx
- libs/ui-lib/lib/common/components/AlertsContextProvider.tsx
- libs/ui-lib/lib/common/features/featureGate.tsx
- libs/ui-lib/lib/common/components/hosts/HostPropertyValidationPopover.tsx
- libs/ui-lib/lib/common/components/hosts/HostStatus.tsx
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-12-17T09:08:18.279Z
Learnt from: jgyselov
Repo: openshift-assisted/assisted-installer-ui PR: 3319
File: libs/ui-lib/lib/cim/components/helpers/toAssisted.ts:199-201
Timestamp: 2025-12-17T09:08:18.279Z
Learning: In libs/ui-lib/lib/cim/components/helpers/toAssisted.ts, the platformType field on agentClusterInstall.spec is guaranteed to always contain a valid PlatformType value (when lowercased), making the type assertion safe without additional runtime validation.
Applied to files:
libs/ui-lib/lib/ocm/components/ui/AssistedUILibVersion.tsxlibs/ui-lib/lib/ocm/store/slices/clusters/selectors.tsx
📚 Learning: 2026-01-07T15:19:02.310Z
Learnt from: rawagner
Repo: openshift-assisted/assisted-installer-ui PR: 3350
File: apps/assisted-ui/package.json:26-27
Timestamp: 2026-01-07T15:19:02.310Z
Learning: The react-router-dom-v5-compat package is designed to work alongside react-router-dom v4 or v5, not replace it. It declares peer dependencies on react-router-dom "4 || 5" and allows incremental migration by providing React Router v6 APIs while the application continues to run on v5. Having both react-router-dom5.x and react-router-dom-v5-compat installed together is the correct and intended configuration.
Applied to files:
apps/assisted-ui/src/components/App.tsx
📚 Learning: 2025-10-21T04:40:36.292Z
Learnt from: linoyaslan
Repo: openshift-assisted/assisted-installer-ui PR: 3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx:55-63
Timestamp: 2025-10-21T04:40:36.292Z
Learning: In libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx, the network reordering logic in the useEffect (swapping clusterNetworks and serviceNetworks based on the primary machine network's IP family) is for UI consistency only. Validation of empty or invalid CIDRs is handled separately by validation schemas, not by the reordering logic.
Applied to files:
libs/ui-lib/lib/ocm/store/slices/clusters/selectors.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/components/ClusterDeploymentHostsSelectionAdvanced.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/hostDiscovery/ClusterDeploymentHostsDiscovery.tsx
📚 Learning: 2025-05-22T09:12:50.632Z
Learnt from: rawagner
Repo: openshift-assisted/assisted-installer-ui PR: 2942
File: libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentManifestsStep.tsx:93-97
Timestamp: 2025-05-22T09:12:50.632Z
Learning: When working with Formik's FieldArray, use the provided array helpers (like `remove()`, `push()`, etc.) instead of directly mutating the values array to ensure proper state tracking and re-rendering.
Applied to files:
libs/ui-lib/lib/common/components/ui/StaticTextField.tsxlibs/ui-lib/lib/common/components/ui/formik/LabelField.tsx
📚 Learning: 2025-12-17T09:16:41.439Z
Learnt from: jgyselov
Repo: openshift-assisted/assisted-installer-ui PR: 3319
File: libs/ui-lib/lib/common/components/hosts/utils.ts:208-210
Timestamp: 2025-12-17T09:16:41.439Z
Learning: In libs/ui-lib/lib/common/components/hosts/utils.ts, the nodeLabels field on Host objects is guaranteed to be valid JSON because it's always constructed using JSON.stringify in libs/ui-lib/lib/cim/components/helpers/toAssisted.ts, making direct JSON.parse safe without additional error handling.
Applied to files:
libs/ui-lib/lib/common/components/ui/formik/LabelField.tsx
📚 Learning: 2025-05-22T07:31:16.062Z
Learnt from: rawagner
Repo: openshift-assisted/assisted-installer-ui PR: 2826
File: libs/ui-lib/lib/cim/components/YamlEditor/YamlEditor.tsx:50-61
Timestamp: 2025-05-22T07:31:16.062Z
Learning: The YamlEditor component in libs/ui-lib/lib/cim/components/YamlEditor/YamlEditor.tsx already handles YAML validation through the submitYamls function in utils.ts, which validates YAML syntax and throws user-friendly error messages if parsing fails.
Applied to files:
libs/ui-lib/lib/cim/components/YamlPreview/YamlPreview.tsx
🧬 Code graph analysis (7)
libs/ui-lib/lib/common/components/ui/formik/CertificatesUploadField.tsx (1)
libs/ui-lib/lib/common/components/ui/formik/types.ts (1)
UploadFieldProps(97-106)
libs/ui-lib/lib/common/components/ui/formik/InputField.tsx (1)
libs/ui-lib/lib/common/components/ui/formik/types.ts (1)
InputFieldProps(66-74)
apps/assisted-ui/src/components/App.tsx (1)
libs/ui-lib/lib/ocm/components/Routes.tsx (1)
UILibRoutes(25-61)
libs/ui-lib/lib/ocm/store/slices/clusters/selectors.tsx (1)
libs/ui-lib/lib/common/components/ui/table/utils.ts (1)
HumanizedSortable(7-11)
libs/ui-lib/lib/common/components/ui/formik/LabelField.tsx (3)
libs/types/accounts-management-service.d.ts (1)
Label(46-56)libs/ui-lib/lib/common/components/ui/formik/utils.ts (1)
getFieldId(15-18)libs/ui-lib/lib/common/hooks/index.ts (1)
useTranslation(3-3)
libs/ui-lib/lib/cim/components/modals/MassApproveAgentModal.tsx (3)
libs/ui-lib/lib/common/components/hosts/tableUtils.tsx (1)
hostnameColumn(59-92)libs/ui-lib/lib/cim/types/k8s/agent.ts (1)
AgentK8sResource(32-74)libs/ui-lib/lib/common/components/hosts/AITable.tsx (1)
TableRow(24-28)
libs/ui-lib/lib/common/components/ui/formik/TextAreaField.tsx (1)
libs/ui-lib/lib/common/components/ui/formik/types.ts (1)
TextAreaFieldProps(88-95)
🪛 Biome (2.1.2)
libs/ui-lib/lib/common/components/ui/formik/LabelField.tsx
[error] 17-17: Shouldn't redeclare 'Label'. Consider to delete it or rename it.
'Label' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (29)
libs/ui-lib/lib/common/components/ui/Toolbar/ToolbarSecondaryGroup.tsx (1)
4-4: Correct React 18 typing fix.This properly addresses the React 18 breaking change where
React.FCno longer implicitly includeschildrenin its props. Explicitly typing withReact.PropsWithChildrenis the standard fix.libs/ui-lib/lib/common/components/ui/formik/TextAreaField.tsx (1)
14-14: LGTM! Correct React 18 type fix.The
PropsWithChildrenwrapper is necessary since React 18'sFCtype no longer implicitly includeschildren. This component useschildren(rendered at line 48), so the type update is appropriate. Based on the provided React documentation, this aligns with the recommended upgrade path.libs/ui-lib/lib/common/components/hosts/AITable.tsx (1)
228-236: LGTM!The change to type the unused
_eventparameter asunknownis a sensible improvement. Since the event is not used in the callback body,unknownis appropriate and safer than an implicitany. This aligns with the PR's goal of improving TypeScript types for React 18 compatibility.libs/ui-lib/lib/common/components/ui/formik/CertificatesUploadField.tsx (1)
17-26: LGTM! Correct React 18 type fix.In React 18,
React.FCno longer implicitly includeschildrenin its type definition. Since this component destructures and renderschildren(line 93), wrappingUploadFieldPropswithReact.PropsWithChildrenis the appropriate fix to maintain type safety.libs/ui-lib/lib/common/components/ui/formik/InputField.tsx (1)
17-21: LGTM!The addition of
React.PropsWithChildren<>is correct for React 18 compatibility. Sincechildrenis destructured and rendered (line 89), explicitly including it in the type is necessary now thatReact.FCno longer implicitly includes children.libs/ui-lib/lib/cim/components/YamlPreview/YamlPreview.tsx (3)
2-4: LGTM!The imports are correctly structured with proper TypeScript types for Monaco editor hooks and the ResizeObserver integration aligns with the PR's pattern of replacing
react-measurewith@react-hook/resize-observer.
17-26: LGTM!The explicit
editor.IStandaloneThemeDatatyping improves type safety and ensures the theme object conforms to Monaco's expected structure.
41-60: LGTM!The component signature update to
React.PropsWithChildren<YamlPreviewProps>aligns with React 18 type patterns. The typed callbacks (EditorDidMount,EditorWillMount) improve type safety, and the ResizeObserver integration correctly tracks container height changes.apps/assisted-ui/src/components/App.tsx (1)
18-27: LGTM!The
BrowserRouter18wrapper is correctly applied, preserving the existingbasenameconfiguration and maintaining the same component hierarchy withCompatRouterfor the incremental migration.libs/ui-lib/lib/common/components/ui/CustomToolbarFilter.tsx (1)
118-123: Cleaner with optional chaining; consider consistency with other portals.The refactor to optional chaining and explicit ternary is cleaner. The
as React.ReactElementcast addresses React 18's stricter typing for fragment children.However, lines 133-137 use similar portal patterns but retain the old
&&style without type assertions. If the type fix is needed here, verify whether those paths might also trigger type errors in certain conditions.libs/ui-lib/lib/cim/components/ClusterDeployment/components/ClusterDeploymentHostsSelectionAdvanced.tsx (3)
5-5: LGTM!The import for
useResizeObserverfrom@react-hook/resize-observeris correct and aligns with the PR's goal of replacing the deprecatedreact-measurelibrary.
93-102: LGTM!The wrapper
divwith the ref provides the measurement target for the resize observer, and the observed width is correctly passed toAgentsSelectionTable. This is the standard pattern for resize observer usage and cleanly replaces the previousMeasurecomponent approach.
61-65: No action needed—AgentsSelectionTableproperly handlesundefinedwidth.The
widthprop is correctly typed as optional (width?: number), and the component uses a defensive guard when consuming it (width && width > 700). When width is undefined,addAllevaluates to false, which simply omits certain columns from the table—no layout issues or errors.libs/ui-lib/lib/cim/components/modals/MassDeleteAgentModal.tsx (3)
33-33: LGTM!The explicit
t<string>generic ensures the return type is narrowed tostring, which aligns with theTableRowheadertitletype requirements and matches the existing pattern intableUtils.tsx.
66-66: LGTM!Consistent application of the
t<string>generic for the status column header title.
88-103: LGTM!The
t<string>generics are correctly applied to the Popover content strings (header, body, footer, and button text), ensuring proper type narrowing for React node children.libs/ui-lib/lib/cim/components/modals/MassApproveAgentModal.tsx (3)
21-24: LGTM!Correct React 18 fix. The
React.PropsWithChildrenwrapper is required because React 18 removed the implicitchildrenprop fromReact.FC. The explicit destructuring ofchildrenand its usage in the component is properly implemented.
29-29: LGTM!Consistent with the
t<string>pattern applied across the codebase for header titles.
54-54: LGTM!Status column header title correctly uses
t<string>for type narrowing.libs/ui-lib/lib/common/components/ui/GridGap.tsx (1)
7-9: LGTM!The
React.PropsWithChildrentyping is the correct fix for React 18, whereReact.FCno longer implicitly includeschildrenin its props type.libs/ui-lib/lib/common/components/AddHosts/AddHostsContext.tsx (1)
13-23: LGTM!Correct React 18 typing fix—explicitly wrapping
AddHostsContextTypewithReact.PropsWithChildrento account forchildrenno longer being implicit inReact.FC.libs/ui-lib/lib/ocm/components/ui/AssistedUILibVersion.tsx (1)
4-11: LGTM!Appropriate React 18 typing update. The component correctly renders children alongside the version metadata.
libs/ui-lib/lib/common/components/hosts/HostsTableDetailContext.tsx (1)
15-29: LGTM!Correct React 18 typing fix for the context provider. The multi-line generic syntax is properly formatted.
libs/ui-lib/lib/common/components/ui/StaticTextField.tsx (3)
22-57: LGTM!Correct React 18 typing for
StaticField. The component properly renders children within the FormGroup.
66-77: LGTM!The
PropsWithChildrentyping is correct. Children will be included in the...propsspread and passed through toStaticField.
83-96: LGTM!Correct React 18 typing for
StaticTextField. The component properly wraps children in the Content component.libs/ui-lib/lib/cim/components/ClusterDeployment/hostDiscovery/ClusterDeploymentHostsDiscovery.tsx (2)
109-124: LGTM!The wrapper div correctly anchors the resize observer, and the width prop is passed cleanly to the table component. This is a good refactor from the render-prop
Measurepattern to a more idiomatic hook-based approach.
71-75: Resize observer implementation correctly handles initial undefined width.The hook setup properly observes the container and updates width on resize. The child component
ClusterDeploymentHostDiscoveryTablegracefully handles the initial undefined state via thewidth && width > 700check (line 80), which safely falses when width is undefined, rendering fewer columns until the observer fires.libs/ui-lib/lib/common/components/ui/formik/LabelField.tsx (1)
65-88: LGTM on the LabelGroup rendering logic.The refactored label rendering using PatternFly's
LabelGroupand editableLabelcomponents is clean. The edit handlers correctly integrate with the PatternFly edit callbacks, and the key generation using${key}__${index}handles duplicate labels appropriately.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
rawagner
force-pushed
the
types_18
branch
2 times, most recently
from
7dbaadd to
8c5540b
Compare
Replace/remove deprecated deps
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/ui-lib/lib/common/components/ui/formik/SelectFieldWithSearch.tsx (1)
247-263: Missing key on outermost element in map.When rendering lists in React, the
keyprop must be on the outermost element returned from the map callback. The shorthand<>fragment syntax doesn't support keys. This can cause reconciliation warnings and unexpected behavior.Suggested fix
selectOptions.map((option, index) => ( - <> + <React.Fragment key={option.value as string}> {option.showDivider && <Divider component="li" key={`divider-${index}`} />} <SelectOption hasCheckbox={isMultiSelect && !option.isAriaDisabled} isSelected={ isMultiSelect && Array.isArray(value) ? value.some((v) => isEqual(v, option.value)) : value === option.value } - key={`option-${index}`} isFocused={focusedItemIndex === index} ref={null} {...option} /> - </> + </React.Fragment> ))
♻️ Duplicate comments (2)
libs/ui-lib/lib/common/components/ui/formik/LabelField.tsx (2)
26-27: Guard against undefinedlabelsto prevent runtime errors.If the form field isn't initialized with an array,
labels.map()at line 63 will throw. Provide a fallback.Suggested fix
- const [{ value: labels }, { touched, error }, { setValue: setLabels, setTouched }] = - useField<string[]>(name); + const [{ value }, { touched, error }, { setValue: setLabels, setTouched }] = + useField<string[]>(name); + const labels = value ?? [];
133-141: Potential race condition:onBlurmay fire before Escape key discard completes.When the user presses Escape,
onDiscardAdd()is called, butonBlur(line 133) fires as the input loses focus during the state update, potentially triggeringonConfirmAdd()before the discard takes effect. Since React state updates are asynchronous, thelabelvalue inonConfirmAddwill still contain the typed text.Suggested fix using a ref to suppress the blur handler
const EditableLabelControl = ({ defaultLabel, onAddLabel }: EditableLabelControlProps) => { const [isEditing, setIsEditing] = React.useState<boolean>(false); const [label, setLabel] = React.useState<string>(''); + const shouldDiscardRef = React.useRef(false); const { t } = useTranslation(); const onConfirmAdd = () => { + if (shouldDiscardRef.current) { + shouldDiscardRef.current = false; + return; + } onAddLabel(label); setIsEditing(false); setLabel(''); }; const onDiscardAdd = () => { + shouldDiscardRef.current = true; setIsEditing(false); setLabel(''); };
🧹 Nitpick comments (1)
libs/ui-lib/package.json (1)
47-47: Align@types/react-domwith@types/reactto avoid type skew.
Consider bumping@types/react-domto match@types/react(18.2.37) and confirmyarn check_typespasses.♻️ Proposed change
- "@types/react-dom": "18.2.15", + "@types/react-dom": "18.2.37",
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (43)
apps/assisted-disconnected-ui/package.jsonapps/assisted-ui/package.jsonapps/assisted-ui/src/components/App.tsxlibs/locales/lib/en/translation.jsonlibs/ui-lib/lib/cim/components/Agent/tableUtils.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/components/ClusterDeploymentHostsSelectionAdvanced.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/customManifests/ConfigMapForm.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/hostDiscovery/ClusterDeploymentHostsDiscovery.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/hostSelection/ClusterDeploymentHostsSelectionAdvanced.tsxlibs/ui-lib/lib/cim/components/InfraEnv/InfraEnvFormPage.tsxlibs/ui-lib/lib/cim/components/YamlPreview/YamlPreview.tsxlibs/ui-lib/lib/cim/components/modals/EditSSHKeyModal.tsxlibs/ui-lib/lib/cim/components/modals/MassApproveAgentModal.tsxlibs/ui-lib/lib/cim/components/modals/MassDeleteAgentModal.tsxlibs/ui-lib/lib/common/components/AddHosts/AddHostsContext.tsxlibs/ui-lib/lib/common/components/AlertsContextProvider.tsxlibs/ui-lib/lib/common/components/clusterWizard/ClusterWizardStepHeader.tsxlibs/ui-lib/lib/common/components/hosts/AITable.tsxlibs/ui-lib/lib/common/components/hosts/HostPropertyValidationPopover.tsxlibs/ui-lib/lib/common/components/hosts/HostStatus.tsxlibs/ui-lib/lib/common/components/hosts/HostsTableDetailContext.tsxlibs/ui-lib/lib/common/components/hosts/TableToolbar.tsxlibs/ui-lib/lib/common/components/hosts/tableUtils.tsxlibs/ui-lib/lib/common/components/storage/DisksTable.tsxlibs/ui-lib/lib/common/components/ui/CustomToolbarFilter.tsxlibs/ui-lib/lib/common/components/ui/GridGap.tsxlibs/ui-lib/lib/common/components/ui/StaticTextField.tsxlibs/ui-lib/lib/common/components/ui/Toolbar/ToolbarSecondaryGroup.tsxlibs/ui-lib/lib/common/components/ui/formik/CertificatesUploadField.tsxlibs/ui-lib/lib/common/components/ui/formik/InputField.tsxlibs/ui-lib/lib/common/components/ui/formik/LabelField.tsxlibs/ui-lib/lib/common/components/ui/formik/NumberInputField.tsxlibs/ui-lib/lib/common/components/ui/formik/SelectFieldWithSearch.tsxlibs/ui-lib/lib/common/components/ui/formik/TextAreaField.tsxlibs/ui-lib/lib/common/components/ui/formik/UploadField.tsxlibs/ui-lib/lib/common/features/featureGate.tsxlibs/ui-lib/lib/ocm/components/SentryErrorMonitorContextProvider.tsxlibs/ui-lib/lib/ocm/components/clusters/ClusterToolbar.tsxlibs/ui-lib/lib/ocm/components/hosts/ModalDialogsContext.tsxlibs/ui-lib/lib/ocm/components/ui/AssistedUILibVersion.tsxlibs/ui-lib/lib/ocm/store/slices/clusters/selectors.tsxlibs/ui-lib/package.jsonpackage.json
💤 Files with no reviewable changes (3)
- package.json
- apps/assisted-ui/package.json
- apps/assisted-disconnected-ui/package.json
🚧 Files skipped from review as they are similar to previous changes (22)
- libs/ui-lib/lib/common/components/ui/formik/UploadField.tsx
- libs/ui-lib/lib/ocm/components/SentryErrorMonitorContextProvider.tsx
- libs/ui-lib/lib/common/components/ui/formik/CertificatesUploadField.tsx
- libs/ui-lib/lib/cim/components/modals/MassDeleteAgentModal.tsx
- libs/ui-lib/lib/common/components/ui/formik/InputField.tsx
- libs/ui-lib/lib/common/features/featureGate.tsx
- apps/assisted-ui/src/components/App.tsx
- libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvFormPage.tsx
- libs/ui-lib/lib/common/components/AlertsContextProvider.tsx
- libs/ui-lib/lib/common/components/hosts/AITable.tsx
- libs/ui-lib/lib/common/components/ui/StaticTextField.tsx
- libs/ui-lib/lib/cim/components/ClusterDeployment/components/ClusterDeploymentHostsSelectionAdvanced.tsx
- libs/ui-lib/lib/cim/components/modals/MassApproveAgentModal.tsx
- libs/ui-lib/lib/cim/components/ClusterDeployment/hostSelection/ClusterDeploymentHostsSelectionAdvanced.tsx
- libs/ui-lib/lib/ocm/store/slices/clusters/selectors.tsx
- libs/ui-lib/lib/cim/components/ClusterDeployment/hostDiscovery/ClusterDeploymentHostsDiscovery.tsx
- libs/ui-lib/lib/common/components/hosts/HostPropertyValidationPopover.tsx
- libs/locales/lib/en/translation.json
- libs/ui-lib/lib/cim/components/modals/EditSSHKeyModal.tsx
- libs/ui-lib/lib/common/components/ui/formik/TextAreaField.tsx
- libs/ui-lib/lib/common/components/ui/CustomToolbarFilter.tsx
- libs/ui-lib/lib/ocm/components/ui/AssistedUILibVersion.tsx
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2026-01-07T15:19:02.310Z
Learnt from: rawagner
Repo: openshift-assisted/assisted-installer-ui PR: 3350
File: apps/assisted-ui/package.json:26-27
Timestamp: 2026-01-07T15:19:02.310Z
Learning: The react-router-dom-v5-compat package is designed to work alongside react-router-dom v4 or v5, not replace it. It declares peer dependencies on react-router-dom "4 || 5" and allows incremental migration by providing React Router v6 APIs while the application continues to run on v5. Having both react-router-dom5.x and react-router-dom-v5-compat installed together is the correct and intended configuration.
Applied to files:
libs/ui-lib/package.json
📚 Learning: 2025-12-17T09:08:18.279Z
Learnt from: jgyselov
Repo: openshift-assisted/assisted-installer-ui PR: 3319
File: libs/ui-lib/lib/cim/components/helpers/toAssisted.ts:199-201
Timestamp: 2025-12-17T09:08:18.279Z
Learning: In libs/ui-lib/lib/cim/components/helpers/toAssisted.ts, the platformType field on agentClusterInstall.spec is guaranteed to always contain a valid PlatformType value (when lowercased), making the type assertion safe without additional runtime validation.
Applied to files:
libs/ui-lib/lib/cim/components/Agent/tableUtils.tsxlibs/ui-lib/lib/cim/components/ClusterDeployment/customManifests/ConfigMapForm.tsx
📚 Learning: 2025-12-17T09:16:41.439Z
Learnt from: jgyselov
Repo: openshift-assisted/assisted-installer-ui PR: 3319
File: libs/ui-lib/lib/common/components/hosts/utils.ts:208-210
Timestamp: 2025-12-17T09:16:41.439Z
Learning: In libs/ui-lib/lib/common/components/hosts/utils.ts, the nodeLabels field on Host objects is guaranteed to be valid JSON because it's always constructed using JSON.stringify in libs/ui-lib/lib/cim/components/helpers/toAssisted.ts, making direct JSON.parse safe without additional error handling.
Applied to files:
libs/ui-lib/lib/cim/components/Agent/tableUtils.tsxlibs/ui-lib/lib/common/components/ui/formik/LabelField.tsxlibs/ui-lib/lib/common/components/hosts/tableUtils.tsxlibs/ui-lib/lib/common/components/hosts/HostStatus.tsx
📚 Learning: 2025-05-22T09:12:50.632Z
Learnt from: rawagner
Repo: openshift-assisted/assisted-installer-ui PR: 2942
File: libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentManifestsStep.tsx:93-97
Timestamp: 2025-05-22T09:12:50.632Z
Learning: When working with Formik's FieldArray, use the provided array helpers (like `remove()`, `push()`, etc.) instead of directly mutating the values array to ensure proper state tracking and re-rendering.
Applied to files:
libs/ui-lib/lib/common/components/ui/formik/LabelField.tsx
📚 Learning: 2025-05-22T07:31:16.062Z
Learnt from: rawagner
Repo: openshift-assisted/assisted-installer-ui PR: 2826
File: libs/ui-lib/lib/cim/components/YamlEditor/YamlEditor.tsx:50-61
Timestamp: 2025-05-22T07:31:16.062Z
Learning: The YamlEditor component in libs/ui-lib/lib/cim/components/YamlEditor/YamlEditor.tsx already handles YAML validation through the submitYamls function in utils.ts, which validates YAML syntax and throws user-friendly error messages if parsing fails.
Applied to files:
libs/ui-lib/lib/cim/components/YamlPreview/YamlPreview.tsx
📚 Learning: 2025-10-20T08:57:04.935Z
Learnt from: linoyaslan
Repo: openshift-assisted/assisted-installer-ui PR: 3190
File: libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx:43-45
Timestamp: 2025-10-20T08:57:04.935Z
Learning: In the file `libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx`, the `getStackTypeLabel` function returns 'Dual-stack' or 'IPv4' because the system currently only supports IPv4 single-stack clusters. IPv6-only single-stack clusters are not supported.
Applied to files:
libs/ui-lib/lib/common/components/hosts/tableUtils.tsx
🧬 Code graph analysis (3)
libs/ui-lib/lib/common/components/ui/formik/NumberInputField.tsx (1)
libs/ui-lib/lib/common/components/ui/formik/types.ts (1)
NumberInputFieldProps(76-86)
libs/ui-lib/lib/common/components/ui/formik/LabelField.tsx (3)
libs/ui-lib/lib/common/hooks/index.ts (1)
useTranslation(3-3)libs/ui-lib/lib/common/components/ui/formik/utils.ts (1)
getFieldId(15-18)libs/types/accounts-management-service.d.ts (1)
Label(46-56)
libs/ui-lib/lib/common/components/hosts/HostStatus.tsx (1)
libs/ui-lib/lib/common/components/hosts/types.ts (2)
HostStatus(81-83)HostStatusProps(85-95)
🔇 Additional comments (29)
libs/ui-lib/lib/cim/components/ClusterDeployment/customManifests/ConfigMapForm.tsx (1)
220-220: Clearer truthiness check for error rendering.
!!isErrormakes the conditional explicit and avoids accidental rendering whenisErroris non-boolean.libs/ui-lib/lib/common/components/hosts/tableUtils.tsx (1)
67-67: LGTM — explicit translation typing is consistent and safe.
No behavioral changes; this tightens types as intended.Also applies to: 103-103, 135-135, 172-172, 191-191, 209-209, 236-236, 261-261, 286-286, 308-308, 387-387
libs/ui-lib/lib/common/components/storage/DisksTable.tsx (1)
42-44: LGTM!The explicit generic type parameter
t<string>(...)correctly ensures the translation function returns astringtype, improving type safety for thebodyContentprop. This is consistent with the broader typing improvements in this PR.libs/ui-lib/lib/common/components/ui/Toolbar/ToolbarSecondaryGroup.tsx (1)
4-5: LGTM for the typing update.
Please confirm this aligns with the repo’s React 18 type expectations (e.g., no TS/lint complaints aboutReact.FC<React.PropsWithChildren>in this codebase).libs/ui-lib/package.json (1)
13-13: Confirm ResizeObserver availability in supported environments.
@react-hook/resize-observerrelies on the nativeResizeObserver. If any supported browsers/environments lack it, add a polyfill or guard to prevent runtime failures.libs/ui-lib/lib/common/components/ui/formik/SelectFieldWithSearch.tsx (1)
166-184: LGTM! Good type safety improvement.Narrowing from
anytounknownaligns with TypeScript best practices and the PR's goal of fixing React 18 types. The function body already handles the unknown type safely with appropriate checks before usage.libs/ui-lib/lib/cim/components/Agent/tableUtils.tsx (5)
47-49: Explicit translation typing looks good.
94-96: Typed translation call is fine.
148-150: LGTM — typing is clearer here.
196-198: No concerns with the typed header title.
236-238: Looks good with the explicit generic.libs/ui-lib/lib/ocm/components/hosts/ModalDialogsContext.tsx (1)
105-108: PropsWithChildren update is fine.libs/ui-lib/lib/common/components/clusterWizard/ClusterWizardStepHeader.tsx (1)
4-4: LGTM — explicit children typing is good.libs/ui-lib/lib/ocm/components/clusters/ClusterToolbar.tsx (1)
9-12: Typing update looks good.libs/ui-lib/lib/common/components/ui/GridGap.tsx (1)
7-8: LGTM — typing/formatting update only.libs/ui-lib/lib/common/components/hosts/TableToolbar.tsx (1)
38-47: LGTM!The
React.PropsWithChildrenwrapper is the correct approach for React 18 compatibility, whereReact.FCno longer implicitly includeschildren. The component properly destructures and renderschildren.libs/ui-lib/lib/common/components/ui/formik/NumberInputField.tsx (1)
18-35: LGTM!The
React.PropsWithChildrenwrapper correctly types thechildrenprop that is destructured and rendered in theSplitItemat line 84. TheforwardRefusage remains compatible with this typing pattern.libs/ui-lib/lib/common/components/hosts/HostsTableDetailContext.tsx (1)
15-29: LGTM!The
React.PropsWithChildrenwrapper is correctly applied to the context provider, enabling proper typing for thechildrenprop that gets rendered inside theHostsTableDetailContext.Provider.libs/ui-lib/lib/common/components/AddHosts/AddHostsContext.tsx (1)
13-23: LGTM!The
React.PropsWithChildrenwrapper correctly types the context provider'schildrenprop, maintaining the standard pattern used across the codebase for React 18 compatibility.libs/ui-lib/lib/common/components/hosts/HostStatus.tsx (2)
186-207: LGTM!The
React.PropsWithChildrenwrapper correctly typesWithHostStatusPopover, which renderschildreninside theButtoncomponent at line 204.
231-243: LGTM!The
React.PropsWithChildrenwrapper is correctly applied toHostStatus. The component useschildrenfor conditional rendering logic (lines 282, 287), and this typing aligns with theHostStatusPropsinterface which doesn't explicitly includechildren.libs/ui-lib/lib/cim/components/YamlPreview/YamlPreview.tsx (4)
1-4: LGTM on import updates.The imports are well-organized:
EditorDidMountandEditorWillMounttypes fromreact-monaco-editor,editornamespace frommonaco-editorfor theme typing, anduseResizeObserverfrom@react-hook/resize-observerto replacereact-measure.
17-33: Good addition of explicit typing for the theme.Typing the theme as
editor.IStandaloneThemeDataimproves type safety and IDE support.
41-60: Clean migration to ResizeObserver-based sizing.The implementation correctly:
- Types the lifecycle callbacks with
EditorDidMountandEditorWillMount- Uses a ref to the container div for observation
- Updates state on resize to drive the editor height
One minor note: the initial render will have
editorHeightasundefineduntil the resize observer fires. This is expected and preferable to rendering with incorrect dimensions.
87-100: Conditional rendering prevents zero-height editor issues.The
!!editorHeightguard ensures the Monaco editor only renders once valid dimensions are available, avoiding layout thrashing. The wrapper div withheight: '100%'and ref correctly captures the available space.libs/ui-lib/lib/common/components/ui/formik/LabelField.tsx (4)
1-16: LGTM!Imports are well-organized and appropriate for the refactored component using PatternFly's LabelGroup.
32-55: LGTM!The handlers correctly implement immutable array updates and properly trigger Formik's touched/validation states.
57-101: LGTM!The LabelGroup rendering is well-structured with proper edit/delete callbacks. The error message display issue from the previous review has been addressed.
144-157: LGTM!The Button implementation is clean with proper accessibility attributes and inline styling.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jgyselov, rawagner The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit a0b51ec
into
openshift-assisted:master
2 participants
Replace/remove deprecated deps
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.