MGMT-20404: Refactor dropdown component import package#2941
Conversation
|
""" WalkthroughThis update systematically refactors dropdown components across multiple files to replace deprecated PatternFly components with their modern equivalents. The changes include swapping out Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DropdownComponent
participant MenuToggle
participant DropdownList
User->>MenuToggle: Clicks toggle
MenuToggle->>DropdownComponent: Toggle open state
DropdownComponent->>DropdownList: Render dropdown items (children)
User->>DropdownList: Selects item
DropdownList->>DropdownComponent: onSelect event
DropdownComponent->>MenuToggle: Update toggle label/state
Suggested labels
Suggested reviewers
Poem
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. 🔧 ESLint
libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentHostsDiscovery.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@openshift-assisted/eslint-config" to extend from. Please check that the name of the config is correct. The config "@openshift-assisted/eslint-config" was referenced from the config file in "/libs/ui-lib/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. libs/ui-lib/lib/cim/components/ClusterDeployment/LabelsSelector.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@openshift-assisted/eslint-config" to extend from. Please check that the name of the config is correct. The config "@openshift-assisted/eslint-config" was referenced from the config file in "/libs/ui-lib/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. libs/ui-lib/lib/cim/components/InfraEnv/AddHostDropdown.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@openshift-assisted/eslint-config" to extend from. Please check that the name of the config is correct. The config "@openshift-assisted/eslint-config" was referenced from the config file in "/libs/ui-lib/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (35)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (33)
⏰ Context from checks skipped due to timeout of 90000ms (6)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ElayAharoni
requested review from
ammont82,
jgyselov and
rawagner
and removed request for
ammont82,
celdrake,
jgyselov and
rawagner
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: 6
🧹 Nitpick comments (3)
libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvOpenShiftVersionDropdown.tsx (1)
36-40: Remove unused dependency to avoid unnecessary renders
useEffectincludestin its dependency list but does not use it in the callback body, causing avoidable re-executions whenever the translation function instance changes.-}, [filteredImages.length, setValue, t, value]); +}, [filteredImages.length, setValue, value]);libs/ui-lib/lib/cim/components/InfraEnv/AddHostDropdown.tsx (1)
61-79: Unify children into aDropdownListfor better a11y & consistency (optional)The first two items (
With Discovery ISO,With iPXE) are rendered directly under<Dropdown>while the BMC items sit inside aDropdownList.
PatternFly 5 guidelines recommend wrapping all items in one or moreDropdownListelements for screen-reader compatibility.Consider:
- <Dropdown - ... - > - {[ - <DropdownItem ... />, - ... - ]} + <Dropdown ...> + <DropdownList> + <DropdownItem ... /> + ... + </DropdownList> + <Divider component="li" /> + <DropdownList id="discovery-bmc" className="ai-discovery-bmc__group"> + ... + </DropdownList> </Dropdown>libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.tsx (1)
117-125: Guard against clicks on non-item contentUsing
event.currentTarget.textContentassumes the clicked element is theDropdownItem.
If you later nest extra markup (e.g., icons) the text may include unwanted whitespace or be undefined.
Consider takinglabeldirectly from a data attribute or the second parameter PatternFly already gives you (val).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentHostsDiscovery.tsx(1 hunks)libs/ui-lib/lib/cim/components/ClusterDeployment/LabelsSelector.tsx(2 hunks)libs/ui-lib/lib/cim/components/InfraEnv/AddHostDropdown.tsx(4 hunks)libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvAgentTable.tsx(1 hunks)libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvOpenShiftVersionDropdown.tsx(3 hunks)libs/ui-lib/lib/cim/components/modals/MassApproveAction.tsx(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx(2 hunks)libs/ui-lib/lib/common/components/hosts/DiskRole.tsx(3 hunks)libs/ui-lib/lib/common/components/hosts/HostToolbarActions.tsx(1 hunks)libs/ui-lib/lib/common/components/hosts/TableToolbar.tsx(3 hunks)libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.tsx(5 hunks)libs/ui-lib/lib/common/components/ui/SimpleDropdown.tsx(3 hunks)libs/ui-lib/lib/common/index.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/CpuArchitectureDropdown.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmBaseDomainField.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/components/FolderDropdown.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/platformIntegration/ExternalPlatformDropdown.tsx(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/platformIntegration/ExternalPlatformDropdown.tsx (2)
libs/types/assisted-installer-service.d.ts (1)
PlatformType(2418-2418)libs/ui-lib-tests/cypress/fixtures/cluster/external-platform-types.ts (1)
externalPlatformTypes(1-23)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: tests
- GitHub Check: circular-deps
- GitHub Check: translation-files
- GitHub Check: format
🔇 Additional comments (49)
libs/ui-lib/lib/cim/components/modals/MassApproveAction.tsx (1)
2-2: Import updated to use non-deprecated PatternFly component.The import for
DropdownItemhas been correctly updated to use the current@patternfly/react-corepackage instead of the deprecated version.libs/ui-lib/lib/common/components/hosts/HostToolbarActions.tsx (1)
2-2: Import updated to use non-deprecated PatternFly component.The import for
DropdownItemhas been correctly updated to use the current@patternfly/react-corepackage instead of the deprecated version.libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvAgentTable.tsx (1)
3-3: Import updated to use non-deprecated PatternFly component.The import for
DropdownItemhas been correctly updated to use the current@patternfly/react-corepackage instead of the deprecated version.libs/ui-lib/lib/common/index.ts (1)
9-9: New utils export added to support dropdown component refactoring.This new export makes utility functions available through the common library's public API, which is necessary to support the PatternFly v5 dropdown component refactoring across the codebase.
libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentHostsDiscovery.tsx (1)
85-92: Good layout restructuring for the AddHostDropdown component.Wrapping the
AddHostDropdowncomponent inside aGridItemwith a specific span improves the layout consistency within the grid system. This change maintains the component's behavior while ensuring it fits properly within the grid layout.libs/ui-lib/lib/cim/components/ClusterDeployment/LabelsSelector.tsx (2)
3-13: Appropriate consolidation of PatternFly imports.The updated imports correctly include the modern PatternFly v5 components needed for dropdown functionality, replacing deprecated components.
133-156: Successfully modernized dropdown implementation.The dropdown has been properly refactored to use PatternFly v5 components:
- Replaced deprecated toggle with
MenuToggle- Updated event handling with
onOpenChangeand explicitonClick- Changed from
dropdownItemsprop to rendering children insideDropdownListThis implementation follows the current PatternFly best practices for dropdown components.
libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmBaseDomainField.tsx (4)
8-12: Correctly updated PatternFly imports.The import statements have been properly updated to include the modern
MenuToggleandMenuToggleElementcomponents.
67-79: Well-implemented dropdown toggle refactoring.The dropdown toggle has been successfully converted from the deprecated approach to use the modern
MenuTogglecomponent with proper event handling and expanded state management.
85-85: Improved type safety for event handler.The event type for
onSelecthas been appropriately refined from a generic synthetic event to a more specific mouse event, improving type safety.
119-121: Properly updated dropdown items rendering.Changed from passing dropdown items via the deprecated
dropdownItemsprop to rendering them as children, which follows current PatternFly patterns.libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/components/FolderDropdown.tsx (4)
2-9: Correctly updated PatternFly imports.The import statements have been properly updated to include the modern PatternFly v5 components.
48-51: Simplified and improved onSelect handler.The event handler has been simplified while maintaining functionality and improving type safety by using a more specific mouse event type.
53-62: Well-implemented modern toggle component.The toggle implementation has been successfully updated to use
MenuTogglewith proper ref handling, click events, and expanded state management.
66-68: Properly updated dropdown rendering.The dropdown now correctly renders items as children rather than using the deprecated
dropdownItemsprop.libs/ui-lib/lib/common/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx (4)
2-10: Correctly updated the imports to use PatternFly v5 components.The imports have been properly updated to include the new
MenuToggle,MenuToggleElement, andDropdownListcomponents from PatternFly v5.
65-71: New event handler signature correctly handles both mouse events and values.The updated event handler now properly accepts optional parameters for both the event and value, which aligns with PatternFly v5's event handling pattern.
73-73: Good separation of toggle handling logic.Creating a dedicated function for toggle handling improves code readability and follows best practices for state management.
78-96: Correctly implemented PatternFly v5 dropdown structure.The dropdown implementation has been properly modernized with:
- Using
MenuToggleinstead of deprecatedDropdownToggle- Adding
onOpenChangehandler for state management- Setting
shouldFocusToggleOnSelectfor better accessibility- Wrapping dropdown items in
DropdownListas children instead of using the deprecateddropdownItemspropThis implementation follows PatternFly v5 best practices.
libs/ui-lib/lib/ocm/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx (4)
2-9: Imports correctly updated to use PatternFly v5 components.The imports have been properly modernized to include
MenuToggleandMenuToggleElementfrom PatternFly v5.
100-104: Event handler type correctly refined.The
onSelectevent handler type has been properly refined to use a more specificReact.MouseEvent<Element, MouseEvent>type, which matches PatternFly v5's expected event types.
117-121: MenuToggle implementation follows PatternFly v5 patterns.The toggle is now correctly implemented as a function that returns a
MenuTogglecomponent with appropriate props for handling click events and showing the expanded state.
130-132: Dropdown structure correctly updated.The dropdown items are now properly passed as children rather than using the deprecated
dropdownItemsprop, aligning with PatternFly v5 patterns.libs/ui-lib/lib/common/components/hosts/DiskRole.tsx (3)
2-2: Imports correctly updated to use PatternFly v5 components.The imports have been properly modernized to include
MenuToggleandMenuToggleElementdirectly from@patternfly/react-core.
84-97: Event handler type correctly refined.The
onSelectevent handler type has been properly refined to use a more specificReact.MouseEvent<Element, MouseEvent>type, aligning with PatternFly v5 patterns.
100-111: MenuToggle implementation follows PatternFly v5 best practices.The toggle is now correctly implemented as a function that returns a
MenuTogglecomponent with appropriate props for handling click events, expanded state, and disabled state. The added className ensures consistent styling.libs/ui-lib/lib/ocm/components/clusterConfiguration/platformIntegration/ExternalPlatformDropdown.tsx (4)
8-13: Imports correctly updated to use PatternFly v5 components.The imports have been properly modernized to include
MenuToggleandMenuToggleElementfrom PatternFly v5.
204-209: DropdownItem correctly updated with value prop.The
valueprop has been added to theDropdownItemcomponents, which aligns with PatternFly v5 expectations for improved selection handling.
244-249: Event handler simplified and properly typed.The event handler has been simplified and now uses the correct
MouseEvent<Element, MouseEvent>type, which matches PatternFly v5's expected event types.
251-261: MenuToggle implementation follows PatternFly v5 patterns.The toggle is now correctly implemented as a function that returns a
MenuTogglecomponent with appropriate props for handling click events, expanded state, and disabled state.libs/ui-lib/lib/ocm/components/clusterConfiguration/CpuArchitectureDropdown.tsx (4)
2-11: Clean up of PatternFly imports looks goodThe consolidated imports from
@patternfly/react-coreand removal of deprecated components aligns with modern PatternFly v5 practices.
128-137: Event handler type refinement is appropriateThe
onSelectcallback now uses a more specificReact.MouseEvent<Element, MouseEvent>type instead of a generic event, which improves type safety.🧰 Tools
🪛 Biome (1.9.4)
[error] 134-134: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
156-165: Correct implementation of MenuToggleThe toggle implementation has been properly updated to use
MenuToggleinstead of the deprecatedDropdownToggle, with appropriate handling of expanded state and onClick events.
169-178: Dropdown children pattern correctly implementedThe
Dropdowncomponent now correctly renders dropdown items as children rather than through the deprecateddropdownItemsprop, following current PatternFly patterns.libs/ui-lib/lib/common/components/ui/SimpleDropdown.tsx (5)
2-8: Import statements updated appropriatelyThe imports have been properly updated to include
MenuToggle,MenuToggleElement, andDropdownPopperPropsfrom@patternfly/react-core, aligning with PatternFly v5 standards.
19-19: Type refinement for menuAppendTo is correctThe prop type has been properly updated to use the PatternFly v5
DropdownPopperProps['appendTo']type.
38-44: Event handler signature updated correctlyThe
onSelectcallback now uses a more specificReact.MouseEvent<Element, MouseEvent>type, improving type safety.
46-58: MenuToggle implementation is completeThe toggle implementation has been properly updated to use
MenuTogglewith the correct props for handling state, includingisFullWidth,onClick,isDisabled, andisExpanded.
61-70: Dropdown popperProps and children correctly implementedThe
Dropdowncomponent now correctly usespopperPropsto set theappendToproperty and renders dropdown items as children, following current PatternFly patterns.libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx (5)
2-2: PatternFly import consolidation is cleanThe imports have been properly updated to include all necessary components from
@patternfly/react-core, removing deprecated imports.
66-66: Added value prop to DropdownItemThe
valueprop has been correctly added to theDropdownItemcomponent, which helps with type safety and selection management.
71-78: Expanded event handler implementationThe
onSelecthandler has been updated to take both an event and value parameter, with appropriate type safety. The function now explicitly sets the display value from the passed value and the form value from the event target's id.
80-89: MenuToggle implementation is correctThe toggle implementation has been properly updated to use
MenuTogglewith appropriate props, includingonClickfor toggling state andisExpandedto reflect open state.
92-94: Dropdown children pattern correctly appliedThe
Dropdowncomponent now correctly renders dropdown items as children rather than using the deprecateddropdownItemsprop, and the redundantnameprop has been removed.libs/ui-lib/lib/common/components/hosts/TableToolbar.tsx (5)
2-20: Import statements updated appropriatelyThe imports have been properly updated to include
MenuToggle,MenuToggleElement,DropdownList, andCheckboxfrom@patternfly/react-core, supporting the updated implementation.
74-78: Actions dropdown toggle correctly implementedThe actions dropdown toggle has been properly updated to use
MenuTogglewith a render function receiving a ref, correctly handling expanded state withisExpandedand toggle behavior withonClick.
80-82: Dropdown children pattern correctly appliedThe actions dropdown now correctly renders items as children rather than using the deprecated
dropdownItemsprop.
95-105: Selection dropdown toggle with checkbox is well implementedThe selection dropdown toggle has been properly updated to use
MenuTogglewith aCheckboxcomponent in the badge prop, correctly handling expanded state and toggle behavior.
108-115: DropdownList wrapper correctly implementedThe dropdown items are now properly wrapped in a
DropdownListcomponent, following current PatternFly patterns for structured dropdown content.
libs/ui-lib/lib/ocm/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx
Outdated
Show resolved
Hide resolved
...lib/lib/ocm/components/clusterConfiguration/platformIntegration/ExternalPlatformDropdown.tsx
Show resolved
Hide resolved
libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvOpenShiftVersionDropdown.tsx
Outdated
Show resolved
Hide resolved
libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.tsx
Outdated
Show resolved
Hide resolved
ElayAharoni
force-pushed
the
Refactor-Dropdown-component-import-package
branch
3 times, most recently
from
987b612 to
165c30d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
libs/ui-lib-tests/cypress/views/clusterDetails.ts(1 hunks)libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentHostsDiscovery.tsx(1 hunks)libs/ui-lib/lib/cim/components/ClusterDeployment/LabelsSelector.tsx(2 hunks)libs/ui-lib/lib/cim/components/InfraEnv/AddHostDropdown.tsx(4 hunks)libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvAgentTable.tsx(1 hunks)libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvOpenShiftVersionDropdown.tsx(3 hunks)libs/ui-lib/lib/cim/components/modals/MassApproveAction.tsx(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx(2 hunks)libs/ui-lib/lib/common/components/hosts/DiskRole.tsx(3 hunks)libs/ui-lib/lib/common/components/hosts/HostToolbarActions.tsx(1 hunks)libs/ui-lib/lib/common/components/hosts/TableToolbar.tsx(3 hunks)libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.tsx(5 hunks)libs/ui-lib/lib/common/components/ui/SimpleDropdown.tsx(3 hunks)libs/ui-lib/lib/common/index.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/CpuArchitectureDropdown.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmBaseDomainField.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/components/FolderDropdown.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/platformIntegration/ExternalPlatformDropdown.tsx(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvAgentTable.tsx
🚧 Files skipped from review as they are similar to previous changes (18)
- libs/ui-lib/lib/common/components/hosts/HostToolbarActions.tsx
- libs/ui-lib/lib/cim/components/modals/MassApproveAction.tsx
- libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentHostsDiscovery.tsx
- libs/ui-lib/lib/common/index.ts
- libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/components/FolderDropdown.tsx
- libs/ui-lib/lib/common/components/ui/SimpleDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx
- libs/ui-lib/lib/cim/components/InfraEnv/AddHostDropdown.tsx
- libs/ui-lib/lib/common/components/hosts/TableToolbar.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/platformIntegration/ExternalPlatformDropdown.tsx
- libs/ui-lib/lib/cim/components/ClusterDeployment/LabelsSelector.tsx
- libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.tsx
- libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvOpenShiftVersionDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmBaseDomainField.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx
- libs/ui-lib/lib/common/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx
- libs/ui-lib/lib/common/components/hosts/DiskRole.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/CpuArchitectureDropdown.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: tests
ElayAharoni
force-pushed
the
Refactor-Dropdown-component-import-package
branch
4 times, most recently
from
5249b57 to
b374a2a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
libs/ui-lib-tests/cypress/views/clusterDetails.ts(3 hunks)libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentHostsDiscovery.tsx(1 hunks)libs/ui-lib/lib/cim/components/ClusterDeployment/LabelsSelector.tsx(2 hunks)libs/ui-lib/lib/cim/components/InfraEnv/AddHostDropdown.tsx(4 hunks)libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvAgentTable.tsx(1 hunks)libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvOpenShiftVersionDropdown.tsx(3 hunks)libs/ui-lib/lib/cim/components/common/CpuArchitectureDropdown.tsx(1 hunks)libs/ui-lib/lib/cim/components/modals/MassApproveAction.tsx(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx(2 hunks)libs/ui-lib/lib/common/components/hosts/DiskRole.tsx(3 hunks)libs/ui-lib/lib/common/components/hosts/HostToolbarActions.tsx(1 hunks)libs/ui-lib/lib/common/components/hosts/TableToolbar.tsx(3 hunks)libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.tsx(5 hunks)libs/ui-lib/lib/common/components/ui/SimpleDropdown.tsx(3 hunks)libs/ui-lib/lib/common/index.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/CpuArchitectureDropdown.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmBaseDomainField.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/components/FolderDropdown.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/platformIntegration/ExternalPlatformDropdown.tsx(4 hunks)
✅ Files skipped from review due to trivial changes (2)
- libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvAgentTable.tsx
- libs/ui-lib/lib/cim/components/common/CpuArchitectureDropdown.tsx
🚧 Files skipped from review as they are similar to previous changes (18)
- libs/ui-lib/lib/common/components/hosts/HostToolbarActions.tsx
- libs/ui-lib/lib/cim/components/modals/MassApproveAction.tsx
- libs/ui-lib/lib/common/index.ts
- libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentHostsDiscovery.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/platformIntegration/ExternalPlatformDropdown.tsx
- libs/ui-lib/lib/cim/components/ClusterDeployment/LabelsSelector.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx
- libs/ui-lib/lib/common/components/hosts/TableToolbar.tsx
- libs/ui-lib/lib/common/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx
- libs/ui-lib/lib/common/components/ui/SimpleDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmBaseDomainField.tsx
- libs/ui-lib/lib/common/components/hosts/DiskRole.tsx
- libs/ui-lib/lib/cim/components/InfraEnv/AddHostDropdown.tsx
- libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.tsx
- libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvOpenShiftVersionDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/CpuArchitectureDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/components/FolderDropdown.tsx
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: translation-files
- GitHub Check: unit-tests
- GitHub Check: tests
- GitHub Check: circular-deps
- GitHub Check: format
- GitHub Check: lint
🔇 Additional comments (3)
libs/ui-lib-tests/cypress/views/clusterDetails.ts (3)
13-18: Update other dropdown getters to follow same pattern.The dropdown interaction has been properly updated to use direct clicks on the field elements instead of nested toggle buttons, aligning with the PatternFly v5 refactoring.
64-75: Good addition of consistent getter methods.The new getter methods for CPU architecture field elements follow a consistent pattern with the other field methods, which improves code maintainability and readability.
152-163: Good addition of consistent getter methods for control plane nodes.These new getter methods for control plane nodes field elements maintain consistency with the pattern established for other dropdown components.
ElayAharoni
force-pushed
the
Refactor-Dropdown-component-import-package
branch
5 times, most recently
from
9b1ba2c to
a4c8f52
Compare
ElayAharoni
force-pushed
the
Refactor-Dropdown-component-import-package
branch
from
1ea857e to
2c45118
Compare
openshift-ci
bot
added
size/XXL
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/DiscoveryImageTypeDropdown.tsx (1)
46-71:⚠️ Potential issue
DropdownItems never propagate a value – useitemId(orvalue) instead ofid
onSelectrelies on the second callback argument (value) to identify which item was chosen, yet none of the renderedDropdownItems setitemId/value. PatternFly 5 only forwardsitemId(PF 4value) – not the DOMid– sovaluewill always beundefined, breaking the cast toDiscoveryImageType, thediscoveryImageTypes[...]lookup, and the parent callback.-<DropdownItem - key="full-iso" - id="full-iso" +<DropdownItem + key="full-iso" + itemId="full-iso" + id="full-iso" // keep if you still need a DOM idApply the same change to
minimal-isoanddiscovery-image-ipxe.
Without this fix users will see an empty label, Formik will get an undefined value, andonChangewill mis-report whether iPXE was chosen.
🧹 Nitpick comments (5)
libs/ui-lib/lib/ocm/components/clusterConfiguration/DiscoveryImageTypeDropdown.tsx (2)
93-103: Expose an accessible label on theMenuToggleScreen-reader users currently hear an empty button because the toggle’s content is dynamic. Add
aria-label(oraria-labelledbypointing to a static<label>):-<MenuToggle +<MenuToggle + aria-label="Select provisioning type"This small addition satisfies WCAG 4.1.2 and avoids an aXe violation.
113-116: Use the boolean supplied byonOpenChangeto avoid stale closures
onOpenChangealready hands you the new expanded state; flipping!isOpenrisks races when multiple state updates fire in quick succession.- onOpenChange={() => setOpen(!isOpen)} + onOpenChange={(nextOpen: boolean) => setOpen(nextOpen)}This keeps React state and PatternFly state in perfect sync.
libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/ExternalPartnerIntegrationsField.ts (3)
3-4: Single source of truth for the root selector
selectoris#form-control__form-input-platform-field, but every helper below uses#form-input-platform-field. Divergent IDs invite brittle tests when markup changes.Consider normalising:
-static readonly selector = '#form-control__form-input-platform-field'; +static readonly selector = '#form-input-platform-field';and update
initaccordingly, or vice-versa.
18-24: Method naming: avoid confusion between “field” and “dropdown”
findExternalPartnerIntegrationsField()returns the clickable input, whilefindDropdown()returns the menu container. Nice separation! To keep intent crystal-clear, prefix withget/openor suffix withInput/Menu(e.g.,getFieldInput,getDropdownMenu) – this is the pattern used in other field helpers.
30-38: Avoid triple-clicking the input when chaining helpers
findDropdownItems()andfindDropdownItem()both call.click()before delegating to other methods that themselves click the input. This results in two clicks per selection and can intermittently close the menu on fast CI runners.-ExternalPartnerIntegrationsField.findExternalPartnerIntegrationsField().click(); -return ExternalPartnerIntegrationsField.findDropdown().find('.pf-v5-c-menu__item'); +return ExternalPartnerIntegrationsField.findDropdown().find('.pf-v5-c-menu__item');Apply the same to
findDropdownItem.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (34)
libs/ui-lib-tests/cypress/fixtures/infra-envs/openshift-versions.ts(1 hunks)libs/ui-lib-tests/cypress/integration/create-sno/0-create-cluster.cy.ts(0 hunks)libs/ui-lib-tests/cypress/integration/day-2/2-adding-hosts.cy.ts(1 hunks)libs/ui-lib-tests/cypress/integration/ui-behaviour/cluster-creation.cy.ts(2 hunks)libs/ui-lib-tests/cypress/integration/use-cases/create-cluster/with-external-partner-integrations.cy.ts(2 hunks)libs/ui-lib-tests/cypress/views/bareMetalDiscovery.ts(1 hunks)libs/ui-lib-tests/cypress/views/clusterDetails.ts(3 hunks)libs/ui-lib-tests/cypress/views/common.ts(1 hunks)libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/ControlPlaneNodesField.ts(1 hunks)libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/CpuArchitectureField.ts(1 hunks)libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/ExternalPartnerIntegrationsField.ts(1 hunks)libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/OpenShiftVersionField.ts(1 hunks)libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentHostsDiscovery.tsx(1 hunks)libs/ui-lib/lib/cim/components/ClusterDeployment/LabelsSelector.tsx(2 hunks)libs/ui-lib/lib/cim/components/InfraEnv/AddHostDropdown.tsx(4 hunks)libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvAgentTable.tsx(1 hunks)libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvOpenShiftVersionDropdown.tsx(3 hunks)libs/ui-lib/lib/cim/components/common/CpuArchitectureDropdown.tsx(1 hunks)libs/ui-lib/lib/cim/components/modals/MassApproveAction.tsx(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx(2 hunks)libs/ui-lib/lib/common/components/hosts/DiskRole.tsx(3 hunks)libs/ui-lib/lib/common/components/hosts/HostToolbarActions.tsx(1 hunks)libs/ui-lib/lib/common/components/hosts/TableToolbar.tsx(3 hunks)libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.css(0 hunks)libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.tsx(6 hunks)libs/ui-lib/lib/common/components/ui/SimpleDropdown.tsx(3 hunks)libs/ui-lib/lib/common/index.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/CpuArchitectureDropdown.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/DiscoveryImageTypeDropdown.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmBaseDomainField.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/components/FolderDropdown.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/platformIntegration/ExternalPlatformDropdown.tsx(4 hunks)
💤 Files with no reviewable changes (2)
- libs/ui-lib-tests/cypress/integration/create-sno/0-create-cluster.cy.ts
- libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.css
✅ Files skipped from review due to trivial changes (1)
- libs/ui-lib-tests/cypress/fixtures/infra-envs/openshift-versions.ts
🚧 Files skipped from review as they are similar to previous changes (29)
- libs/ui-lib/lib/common/components/hosts/HostToolbarActions.tsx
- libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvAgentTable.tsx
- libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/ControlPlaneNodesField.ts
- libs/ui-lib/lib/common/index.ts
- libs/ui-lib/lib/cim/components/modals/MassApproveAction.tsx
- libs/ui-lib-tests/cypress/integration/day-2/2-adding-hosts.cy.ts
- libs/ui-lib-tests/cypress/views/bareMetalDiscovery.ts
- libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentHostsDiscovery.tsx
- libs/ui-lib/lib/cim/components/common/CpuArchitectureDropdown.tsx
- libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/OpenShiftVersionField.ts
- libs/ui-lib-tests/cypress/views/common.ts
- libs/ui-lib/lib/common/components/hosts/TableToolbar.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/components/FolderDropdown.tsx
- libs/ui-lib-tests/cypress/integration/ui-behaviour/cluster-creation.cy.ts
- libs/ui-lib/lib/common/components/hosts/DiskRole.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx
- libs/ui-lib/lib/cim/components/ClusterDeployment/LabelsSelector.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/CpuArchitectureDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/platformIntegration/ExternalPlatformDropdown.tsx
- libs/ui-lib/lib/cim/components/InfraEnv/AddHostDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmBaseDomainField.tsx
- libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.tsx
- libs/ui-lib/lib/common/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx
- libs/ui-lib-tests/cypress/integration/use-cases/create-cluster/with-external-partner-integrations.cy.ts
- libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvOpenShiftVersionDropdown.tsx
- libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/CpuArchitectureField.ts
- libs/ui-lib-tests/cypress/views/clusterDetails.ts
- libs/ui-lib/lib/common/components/ui/SimpleDropdown.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/DiscoveryImageTypeDropdown.tsx (1)
libs/ui-lib/lib/common/types/clusters.ts (1)
DiscoveryImageType(75-75)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: tests
- GitHub Check: translation-files
- GitHub Check: circular-deps
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: format
libs/ui-lib/lib/ocm/components/clusterConfiguration/DiscoveryImageTypeDropdown.tsx
Show resolved
Hide resolved
ElayAharoni
force-pushed
the
Refactor-Dropdown-component-import-package
branch
from
2c45118 to
a5591e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
libs/ui-lib/lib/common/components/hosts/DiskRole.tsx (1)
127-128: DropdownList implementation matches PatternFly v5 patterns.The dropdown items are now correctly wrapped in a
DropdownListcomponent as suggested in the previous review comment. This structure complies with PatternFly v5 patterns.
🧹 Nitpick comments (1)
libs/ui-lib/lib/common/components/hosts/DiskRole.tsx (1)
111-111: Potential redundant state toggling in onClick and onOpenChange.Both the
onClickhandler inMenuToggleand theonOpenChangehandler inDropdownare toggling the dropdown's open state. This redundancy could potentially lead to unexpected behavior.Consider using the
onOpenChangehandler exclusively to manage the open state:<MenuToggle variant="plainText" ref={toggleRef} isFullWidth - onClick={() => setOpen(!isOpen)} isExpanded={isOpen} isDisabled={isDisabled} > {currentRoleLabel} </MenuToggle>And update the
onOpenChangehandler to:<Dropdown - onOpenChange={() => setOpen(!isOpen)} + onOpenChange={(isOpen) => setOpen(isOpen)} onSelect={onSelect} toggle={toggle} isOpen={isOpen} isPlain >Also applies to: 121-121
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (34)
libs/ui-lib-tests/cypress/fixtures/infra-envs/openshift-versions.ts(1 hunks)libs/ui-lib-tests/cypress/integration/create-sno/0-create-cluster.cy.ts(0 hunks)libs/ui-lib-tests/cypress/integration/day-2/2-adding-hosts.cy.ts(1 hunks)libs/ui-lib-tests/cypress/integration/ui-behaviour/cluster-creation.cy.ts(2 hunks)libs/ui-lib-tests/cypress/integration/use-cases/create-cluster/with-external-partner-integrations.cy.ts(2 hunks)libs/ui-lib-tests/cypress/views/bareMetalDiscovery.ts(1 hunks)libs/ui-lib-tests/cypress/views/clusterDetails.ts(3 hunks)libs/ui-lib-tests/cypress/views/common.ts(1 hunks)libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/ControlPlaneNodesField.ts(1 hunks)libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/CpuArchitectureField.ts(1 hunks)libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/ExternalPartnerIntegrationsField.ts(1 hunks)libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/OpenShiftVersionField.ts(1 hunks)libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentHostsDiscovery.tsx(1 hunks)libs/ui-lib/lib/cim/components/ClusterDeployment/LabelsSelector.tsx(2 hunks)libs/ui-lib/lib/cim/components/InfraEnv/AddHostDropdown.tsx(4 hunks)libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvAgentTable.tsx(1 hunks)libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvOpenShiftVersionDropdown.tsx(3 hunks)libs/ui-lib/lib/cim/components/common/CpuArchitectureDropdown.tsx(1 hunks)libs/ui-lib/lib/cim/components/modals/MassApproveAction.tsx(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx(2 hunks)libs/ui-lib/lib/common/components/hosts/DiskRole.tsx(3 hunks)libs/ui-lib/lib/common/components/hosts/HostToolbarActions.tsx(1 hunks)libs/ui-lib/lib/common/components/hosts/TableToolbar.tsx(3 hunks)libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.css(0 hunks)libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.tsx(6 hunks)libs/ui-lib/lib/common/components/ui/SimpleDropdown.tsx(3 hunks)libs/ui-lib/lib/common/index.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/CpuArchitectureDropdown.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/DiscoveryImageTypeDropdown.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmBaseDomainField.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/components/FolderDropdown.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/platformIntegration/ExternalPlatformDropdown.tsx(4 hunks)
💤 Files with no reviewable changes (2)
- libs/ui-lib-tests/cypress/integration/create-sno/0-create-cluster.cy.ts
- libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.css
✅ Files skipped from review due to trivial changes (1)
- libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvAgentTable.tsx
🚧 Files skipped from review as they are similar to previous changes (30)
- libs/ui-lib/lib/cim/components/modals/MassApproveAction.tsx
- libs/ui-lib/lib/common/components/hosts/HostToolbarActions.tsx
- libs/ui-lib/lib/common/index.ts
- libs/ui-lib-tests/cypress/integration/day-2/2-adding-hosts.cy.ts
- libs/ui-lib-tests/cypress/views/bareMetalDiscovery.ts
- libs/ui-lib-tests/cypress/views/common.ts
- libs/ui-lib-tests/cypress/fixtures/infra-envs/openshift-versions.ts
- libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentHostsDiscovery.tsx
- libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/ControlPlaneNodesField.ts
- libs/ui-lib/lib/cim/components/common/CpuArchitectureDropdown.tsx
- libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/OpenShiftVersionField.ts
- libs/ui-lib-tests/cypress/integration/use-cases/create-cluster/with-external-partner-integrations.cy.ts
- libs/ui-lib-tests/cypress/integration/ui-behaviour/cluster-creation.cy.ts
- libs/ui-lib/lib/cim/components/InfraEnv/InfraEnvOpenShiftVersionDropdown.tsx
- libs/ui-lib/lib/common/components/hosts/TableToolbar.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmBaseDomainField.tsx
- libs/ui-lib/lib/common/components/clusterConfiguration/ControlPlaneNodesDropdown.tsx
- libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/CpuArchitectureField.ts
- libs/ui-lib/lib/cim/components/ClusterDeployment/LabelsSelector.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/components/FolderDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/CpuArchitectureDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/DiscoveryImageTypeDropdown.tsx
- libs/ui-lib-tests/cypress/views/forms/ClusterDetails/Fields/ExternalPartnerIntegrationsField.ts
- libs/ui-lib/lib/ocm/components/clusterConfiguration/platformIntegration/ExternalPlatformDropdown.tsx
- libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.tsx
- libs/ui-lib/lib/cim/components/InfraEnv/AddHostDropdown.tsx
- libs/ui-lib/lib/common/components/ui/SimpleDropdown.tsx
- libs/ui-lib-tests/cypress/views/clusterDetails.ts
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: lint
- GitHub Check: translation-files
- GitHub Check: unit-tests
- GitHub Check: tests
- GitHub Check: circular-deps
- GitHub Check: format
🔇 Additional comments (3)
libs/ui-lib/lib/common/components/hosts/DiskRole.tsx (3)
2-8: Import refactoring successfully updates to PatternFly v5 components.The imports have been correctly updated to use the latest PatternFly v5 components, including
DropdownList,MenuToggle, andMenuToggleElement. This aligns with the PR objective of updating all dropdown components to use the PatternFly v5 versions.
91-91: Event type is now properly specified.The event parameter type in the
onSelectcallback has been improved from a generic event to a more specificReact.MouseEvent<Element, MouseEvent>, enhancing type safety and code clarity.
106-117: Toggle implementation correctly follows PatternFly v5 patterns.The toggle implementation has been successfully refactored to use
MenuToggleinstead of the deprecatedDropdownToggle. The suggestedvariant="plainText"from the previous review has been correctly applied.
ElayAharoni
force-pushed
the
Refactor-Dropdown-component-import-package
branch
from
a5591e3 to
5d6a088
Compare
ElayAharoni
force-pushed
the
Refactor-Dropdown-component-import-package
branch
3 times, most recently
from
148def5 to
9a2bbfd
Compare
|
@ElayAharoni: This pull request references MGMT-20404 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
ElayAharoni
force-pushed
the
Refactor-Dropdown-component-import-package
branch
from
9a2bbfd to
2acfea9
Compare
|
@ElayAharoni: This pull request references MGMT-20404 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ElayAharoni, jgyselov 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-ci
bot
added
the
approved
openshift-merge-bot
bot
merged commit a11708e
into
openshift-assisted:master
|
/cherry-pick releases/v2.15-cim |
|
@jgyselov: new pull request created: #2966 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
4 participants
https://issues.redhat.com/browse/MGMT-20404
changed all the dropdown component usages to use the patternfly v5 component and not hte deprecated one
Summary by CodeRabbit