CONSOLE-4709: Remove kebab factory uses from default-resouce list and details pages

[vikram-raj]

Remove the use of the kebab factory from the default-resource list and the details page

• default-resource.tsx

Changes from the big PR #15520

[openshift-ci-robot]

@vikram-raj: This pull request references CONSOLE-4709 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.21.0" version, but no target version was set.

In response to this:

Remove the use of the kebab factory from the default-resource list and the details page

• default-resource.tsx

Changes from the big PR #15520

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.

[coderabbitai]

Walkthrough

Added an appendTo prop to ActionMenu; removed exported PowerOffHostModalProps1; refactored default resource actions to use an internal ResourceActionsMenu and added a customActionMenu prop to DetailsPage.

Changes

Cohort / File(s)	Summary
Action Menu Enhancement frontend/packages/console-shared/src/components/actions/menu/ActionMenu.tsx	Added appendTo?: React.ComponentProps<typeof Popper>['appendTo'] to ActionMenuProps; destructured appendTo from props and forwarded it to the Popper usage as `appendTo={appendTo
Modal Props Cleanup frontend/packages/metal3-plugin/src/components/modals/PowerOffHostModal.tsx	Removed exported type PowerOffHostModalProps1. Updated PowerOffHostModalProps to intersect with ModalComponentProps (public signature now includes & ModalComponentProps); internal component logic unchanged.
Resource Actions Refactoring frontend/public/components/default-resource.tsx, frontend/public/components/factory/details.tsx	Replaced inline ResourceKebab usage with an internal ResourceActionsMenu that composes useCommonResourceActions and renders via ActionMenu (accepts variant and appendTo). DetailsPage gained a new customActionMenu prop and now prefers extension-provided action menus in details view, falling back to ResourceActionsMenu.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Check appendTo typing and runtime fallback to containerRef.current in ActionMenu.tsx.
• Confirm no remaining consumers of PowerOffHostModalProps1 and that the ModalComponentProps intersection doesn't break call sites.
• Validate ResourceActionsMenu wiring and customActionMenu integration in list/details flows, including variant and appendTo behavior for dropdown vs kebab rendering.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[vikram-raj]

/cc @yapei

For QE review

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

frontend/packages/console-shared/src/components/actions/menu/ActionMenu.tsx (1)

17-18: Guard Popper appendTo against null and consider impact of new default

The new appendTo prop is useful, but this expression:

appendTo={appendTo || containerRef.current}

can pass null to Popper on initial render (before containerRef is set), even though the union type doesn’t include null, and it also changes the default behavior from Popper’s internal default to always using the local container.

To keep behavior safer and closer to the previous default, consider:

-        <Popper
-          triggerRef={toggleRef}
-          popper={menu}
-          placement="bottom-end"
-          isVisible={isOpen}
-          appendTo={appendTo || containerRef.current}
-        />
+        <Popper
+          triggerRef={toggleRef}
+          popper={menu}
+          placement="bottom-end"
+          isVisible={isOpen}
+          appendTo={appendTo ?? containerRef.current ?? undefined}
+        />

This avoids ever passing null and only overrides Popper’s default when a container actually exists. Please verify this matches the expectations for Popper’s appendTo semantics in the PatternFly version you’re using.

Also applies to: 27-28, 103-109

frontend/public/components/default-resource.tsx (1)

392-408: Avoid double-fetching the same resource in DefaultDetailsPage

DefaultDetailsPage now calls useK8sGet to fetch resource for ResourceActionsMenu, while DetailsPage itself is already responsible for loading the object passed to its child pages. This means the same resource is fetched twice on details pages, which is unnecessary overhead and adds complexity.

A more scalable design would be to let customActionMenu be a render function that receives the loaded obj from DetailsPage, e.g., customActionMenu={(obj) => <ResourceActionsMenu resource={obj} ... />}, avoiding the extra hook and API call entirely.

Not a blocker for this PR, but worth considering for a follow-up to keep the data flow and network usage lean.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 1fe9940 and 00f0fd2.

📒 Files selected for processing (3)

• frontend/packages/console-shared/src/components/actions/menu/ActionMenu.tsx (3 hunks)
• frontend/packages/metal3-plugin/src/components/modals/PowerOffHostModal.tsx (0 hunks)
• frontend/public/components/default-resource.tsx (4 hunks)

💤 Files with no reviewable changes (1)

• frontend/packages/metal3-plugin/src/components/modals/PowerOffHostModal.tsx

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• frontend/packages/console-shared/src/components/actions/menu/ActionMenu.tsx
• frontend/public/components/default-resource.tsx

🔇 Additional comments (1)

frontend/public/components/default-resource.tsx (1)

53-56: ResourceActionsMenu abstraction looks good

The ResourceActionsMenu wrapper around useCommonResourceActions and ActionMenu cleanly centralizes default actions for a resource and keeps the call sites simple. No issues from a readability or maintainability standpoint here.

Also applies to: 81-89

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

frontend/public/components/default-resource.tsx (2)

53-56: Avoid duplicating ActionMenu prop types in ResourceActionsMenu

The implementation looks good, but variant/appendTo types are re-declared here and can drift from ActionMenu over time. You can tighten coupling and drop the extra spread by deriving props from ActionMenu directly:

-const ResourceActionsMenu: React.FC<{
-  resource: K8sResourceKind;
-  variant: ActionMenuVariant;
-  appendTo?: HTMLElement | (() => HTMLElement) | 'inline';
-}> = ({ resource, variant, appendTo }) => {
-  const common = useCommonResourceActions(kindObj(referenceFor(resource)), resource);
-  const menuActions = [...common];
-  return <ActionMenu actions={menuActions} variant={variant} appendTo={appendTo} />;
-};
+type ResourceActionsMenuProps = {
+  resource: K8sResourceKind;
+} & Pick<React.ComponentProps<typeof ActionMenu>, 'variant' | 'appendTo'>;
+
+const ResourceActionsMenu: React.FC<ResourceActionsMenuProps> = ({
+  resource,
+  variant,
+  appendTo,
+}) => {
+  const common = useCommonResourceActions(kindObj(referenceFor(resource)), resource);
+  return <ActionMenu actions={common} variant={variant} appendTo={appendTo} />;
+};

This keeps ResourceActionsMenu in sync if ActionMenu’s public API evolves and avoids an unnecessary array copy.

Also applies to: 81-89

---

392-408: Consider avoiding a second fetch/watch of the same resource in DefaultDetailsPage

Wiring customActionMenu via:

const [resource, resourceLoaded, resourceError] = useK8sGet(...);

customActionMenu={
  resourceLoaded && !resourceError ? (
    <ResourceActionsMenu resource={resource} variant={ActionMenuVariant.DROPDOWN} />
  ) : null
}

is straightforward, but if DetailsPage already loads/watches this resource internally, this introduces a second fetch/watch for the same object on the details view.

If that’s the case, consider a follow-up refactor where DetailsPage exposes the loaded resource (e.g., via a render prop or by passing it into customActionMenu) so the action menu can reuse the existing data source instead of calling useK8sGet again.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 00f0fd2 and eecfd0a.

📒 Files selected for processing (3)

• frontend/packages/console-shared/src/components/actions/menu/ActionMenu.tsx (3 hunks)
• frontend/packages/metal3-plugin/src/components/modals/PowerOffHostModal.tsx (0 hunks)
• frontend/public/components/default-resource.tsx (4 hunks)

💤 Files with no reviewable changes (1)

• frontend/packages/metal3-plugin/src/components/modals/PowerOffHostModal.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

• frontend/packages/console-shared/src/components/actions/menu/ActionMenu.tsx

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• frontend/public/components/default-resource.tsx

🔇 Additional comments (1)

frontend/public/components/default-resource.tsx (1)

227-231: Kebab appendTo handling is now safe and robust

Using document.getElementById('popper-container') ?? document.body ensures appendTo is never null, so Popper always has a valid container even if the dedicated popper node is missing. This resolves the earlier nullability issue while preserving the desired placement behavior.

[vikram-raj]

/label px-approved
/label docs-approved

[vikram-raj]

/retest

[yapei]

for some resources, kebab action items differ from action menus, take Prometheus for example, on Prometheus list page we can see Edit Pod count is shown in kebab list however it is not in Actions dropdown on details page

prometheus-kebab-action-mismatch.mov

The other resource I'm seeing different menu items is Service Monitor

[yapei]

some other CustomResource without dedicated list/details page for example EtcdBackup, both kebab and actions are showing and behaves correctly

[vikram-raj]

@yapei ServiceMonitor is covered in #15721. Looks like it was a bug in main

[vikram-raj]

@yapei Good catch. Resolved the Prometheus issue action issue on the details page.

[yapei]

The prometheus mismatch issue have been fixed with latest code
/verified by @yapei

[openshift-ci-robot]

@yapei: This PR has been marked as verified by @yapei.

In response to this:

The prometheus mismatch issue have been fixed with latest code
/verified by @yapei

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.

[vikram-raj]

I've updated the props type according to the review comment. No change in feature.

/verified by @yapei

[openshift-ci-robot]

@vikram-raj: This PR has been marked as verified by @yapei.

In response to this:

I've updated the props type according to the review comment. No change in feature.

/verified by @yapei

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.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

frontend/public/components/default-resource.tsx (1)

393-421: Consider aligning LazyActionMenu context key with the extension guard

resourceProviderGuard filters extensions by props.kind, but LazyActionMenu receives context={{ [referenceFor(obj)]: obj }}. If referenceFor(obj) ever differs from props.kind, extensions that passed the guard may not find their object in context. You might want to key the context off props.kind (or at least fall back to it) for consistency with the guard and with the list view pattern.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 293bad0 and e3d453a.

📒 Files selected for processing (3)

• frontend/packages/console-shared/src/components/actions/menu/ActionMenu.tsx (3 hunks)
• frontend/packages/metal3-plugin/src/components/modals/PowerOffHostModal.tsx (0 hunks)
• frontend/public/components/default-resource.tsx (4 hunks)

💤 Files with no reviewable changes (1)

• frontend/packages/metal3-plugin/src/components/modals/PowerOffHostModal.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

• frontend/packages/console-shared/src/components/actions/menu/ActionMenu.tsx

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• frontend/public/components/default-resource.tsx

🔇 Additional comments (2)

frontend/public/components/default-resource.tsx (2)

53-56: ResourceActionsMenu abstraction looks sound and type-safe

Using useCommonResourceActions plus a thin ResourceActionsMenu wrapper, typed via Pick<React.ComponentProps<typeof ActionMenu>, 'variant' | 'appendTo'>, keeps the public API small and aligns with the existing ActionMenu contract. This centralizes default resource actions cleanly without leaking implementation details.

Also applies to: 80-92

---

225-239: Kebab actions migration with appendTo fallback looks correct

The switch to <ResourceActionsMenu> for the row actions, combined with appendTo={document.getElementById('popper-container') ?? document.body}, avoids the earlier null issue and ensures Popper always has a valid container while keeping extension-based actions (LazyActionMenu) intact.

[logonoff]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: logonoff, vikram-raj

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• frontend/OWNERS [logonoff,vikram-raj]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD 86c7b34 and 2 for PR HEAD e3d453a in total

[openshift-ci]

@vikram-raj: all tests passed!

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.