Fixes #39349 - implement new button structure with kebab menu by andreilakatos · Pull Request #817 · theforeman/foreman-tasks

andreilakatos · 2026-05-22T16:22:24Z

Lukshio

Just some nitpick to reduce brace hell

Lukshio · 2026-06-24T15:32:42Z

Rebase + CI failure fix needed

Lukshio · 2026-06-25T09:03:17Z

@andreilakatos Buttons should be on the title level

Figma:

Actual:

…details page

andreilakatos · 2026-06-25T14:38:50Z

@andreilakatos Buttons should be on the title level

Figma:

Actual:

andreilakatos · 2026-06-25T18:11:28Z

@coderabbitai full review

coderabbitai · 2026-06-25T18:11:34Z

✅ Action performed

Full review finished.

coderabbitai · 2026-06-25T18:21:20Z

Walkthrough

Task details rendering was split into a simplified task content component, a new header component, and updated route wiring. The task action controls now live in an overflow dropdown, and the page passes the task action and callbacks into the new header. Route imports, tests, and component registry entries were updated to use the ShowTaskDetails module root.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

theforeman/foreman-tasks#815 — Introduced the React TaskDetails page wiring that this PR refactors further with a separate header and updated task actions.

Suggested reviewers

adamruzicka
MariaAga

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main UI change: replacing task actions with a kebab menu.
Description check	✅ Passed	The screenshot-based description is related to the task details button/menu UI changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

❤️ Share

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

coderabbitai

Actionable comments posted: 5

🧹 Nitpick comments (1)

webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js (1)
43-47: 🎯 Functional Correctness | 🔵 Trivial

Check React version compatibility for defaultProps

Based on the project dependencies (@theforeman/vendor >= 12.1.1) and the timeline of The Foreman ecosystem, the project relies on React 16.x during that vendor version range. In React 16, defaultProps on function components are supported, unlike in React 18/19+ where they are deprecation targets or ignored.

The code using defaultProps on function components (TitleComponent and TaskDetailsPage) is valid for the current React version in this repository. While migrating to parameter defaults (e.g., ({ action = '', state = '', result = '' })) is a recommended modernization step for future-proofing, it is not strictly required by the current runtime version.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js` around
lines 43 - 47, The use of defaultProps on the function components TitleComponent
and TaskDetailsPage is compatible with the repository’s current React 16
runtime, so no compatibility-driven refactor is required here. Keep the existing
defaultProps definitions unless you are intentionally doing a broader
modernization pass; if so, update the component signatures consistently rather
than changing only one of them.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js`:
- Around line 16-21: The Task component test only verifies the TaskInfo metadata
path and does not lock in the new header/body split. Update the Task.test.js
case around Task to add a negative assertion that the cancel/overflow action
controls are not rendered, using the Task render and any existing control
labels/test identifiers tied to TaskButtons or TaskDetailsHeader. This ensures
the test fails if action controls are reintroduced under the details body
instead of staying in the header.

In `@webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js`:
- Around line 145-150: The overflow dropdown in TaskButtons is seeding its menu
from stale state because SimpleDropdown is using initialItems instead of a
controlled items prop. Update the TaskButtons render path that builds
overflowItems so the dropdown re-renders when task-related props like
taskReload, canEdit, and state change; either pass the live items through
SimpleDropdown’s items prop or switch this menu to PatternFly’s Dropdown
component with controlled content.

In
`@webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js`:
- Around line 204-205: The current assertions in TaskDetailsPage.test.js are too
broad and can pass even when the header layout is broken, because TaskInfo also
renders matching icons in the body. Update the Running/Error and cancel-button
checks to query within the title row or header container instead of using global
screen queries, so the test validates the new header placement; use the existing
header/title-row wrapper and keep the assertions scoped there across the
affected blocks.

In `@webpack/ForemanTasks/Routes/ShowTaskDetails/index.js`:
- Around line 41-45: `mapStateToProps` in ShowTaskDetails is using a separate
task-id source via `getTaskID`, which can diverge from the router param used by
`TaskDetailsPage`. Update the `mapStateToProps`/`connect` flow so the only task
id comes from `match.params.id`, and pass that same value through to the page
and any action callbacks instead of reading `window.location.pathname` or
re-parsing it elsewhere.

In `@webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js`:
- Around line 60-61: Move the trailing props spread in TaskDetailsHeader so the
derived resumable and cancellable values are applied after any connected values
from ShowTaskDetails/index.js; currently the spread overwrites the local
executionPlan-based flags before TaskButtons sees them. Update the
TaskDetailsHeader props ordering around the TaskButtons call so the computed
resumable/cancellable values win, and apply the same ordering fix anywhere else
in the component where the spread appears.

---

Nitpick comments:
In `@webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js`:
- Around line 43-47: The use of defaultProps on the function components
TitleComponent and TaskDetailsPage is compatible with the repository’s current
React 16 runtime, so no compatibility-driven refactor is required here. Keep the
existing defaultProps definitions unless you are intentionally doing a broader
modernization pass; if so, update the component signatures consistently rather
than changing only one of them.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: db98c71b-0085-4838-b788-a6ca2d46889d

📥 Commits

Reviewing files that changed from the base of the PR and between 16506cc and 81b5e48.

📒 Files selected for processing (15)

webpack/ForemanTasks/Components/TaskDetails/Components/Task.js
webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js
webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js
webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js
webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/TaskButtons.test.js
webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js
webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js
webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.scss
webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsPage.js
webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsHeader.test.js
webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js
webpack/ForemanTasks/Routes/ShowTaskDetails/index.js
webpack/Routes/routes.js
webpack/Routes/routes.test.js
webpack/index.js

💤 Files with no reviewable changes (3)

webpack/index.js
webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js
webpack/ForemanTasks/Components/TaskDetails/tests/TaskDetails.test.js

…API error

Lukshio · 2026-06-30T09:44:08Z

+} from '@patternfly/react-core';
+import { translate as __ } from 'foremanReact/common/I18n';
+import { STATUS } from 'foremanReact/constants';
+import { usePermissions } from 'foremanReact/common/hooks/Permissions/permissionHooks';


Non blocking - Permissions are also handled by API calls and Page which renders error like this one when user tries to access without permission.

Lukshio · 2026-06-30T11:06:07Z

Now it's spinning but is off center and overflowing

Screencast_20260630_130449.mp4

andreilakatos · 2026-06-30T12:37:38Z

Now it's spinning but is off center and overflowing

Screencast_20260630_130449.mp4

Issue resolved 😄

Lukshio

lgtm, tested

Lukshio · 2026-06-30T13:22:41Z

Thanks @andreilakatos

pr-processor Bot added Not yet reviewed Waiting on contributor labels May 22, 2026

andreilakatos mentioned this pull request May 22, 2026

Fixes #39350 - implement expandable rows for task info #818

Open

Lukshio self-assigned this May 25, 2026

Lukshio reviewed Jun 4, 2026

View reviewed changes

Comment thread webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js

pr-processor Bot removed the Not yet reviewed label Jun 4, 2026

andreilakatos force-pushed the taskdetails-pf5-update-buttons branch from 2cfebe6 to d243b13 Compare June 9, 2026 09:43

pr-processor Bot added the Needs re-review label Jun 9, 2026

andreilakatos force-pushed the taskdetails-pf5-update-buttons branch from d243b13 to 0a8fc4e Compare June 18, 2026 14:58

pr-processor Bot removed the Waiting on contributor label Jun 18, 2026

Lukshio reviewed Jun 24, 2026

View reviewed changes

Comment thread webpack/ForemanTasks/Components/TaskDetails/Components/Task.js Outdated

pr-processor Bot removed the Needs re-review label Jun 24, 2026

andreilakatos force-pushed the taskdetails-pf5-update-buttons branch 3 times, most recently from b05459d to 1ebbd20 Compare June 24, 2026 17:02

Fixes #39349 - implement new button structure with kebab menu

e7f2651

andreilakatos force-pushed the taskdetails-pf5-update-buttons branch from 1ebbd20 to e7f2651 Compare June 24, 2026 17:03

Refs #39349 - move buttons to page title, provide state props to the …

81b5e48

…details page

coderabbitai Bot reviewed Jun 25, 2026

View reviewed changes

andreilakatos force-pushed the taskdetails-pf5-update-buttons branch from f2beb1a to dc76c88 Compare June 26, 2026 12:07

Refs #39349 - route task id via props and align TaskDetails RTL tests

35c742b

andreilakatos force-pushed the taskdetails-pf5-update-buttons branch from dc76c88 to 35c742b Compare June 26, 2026 13:25

Refs #39349 - hide task header actions without view permission or on …

e42cc37

…API error

Lukshio reviewed Jun 30, 2026

View reviewed changes

Refs #39349 - move task buttons css to a separate file

ace21c1

andreilakatos force-pushed the taskdetails-pf5-update-buttons branch from 66baf87 to ace21c1 Compare June 30, 2026 12:00

Lukshio approved these changes Jun 30, 2026

View reviewed changes

Lukshio merged commit 69a1b43 into theforeman:master Jun 30, 2026
26 of 28 checks passed

Uh oh!

Conversation

andreilakatos commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lukshio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Lukshio commented Jun 24, 2026

Uh oh!

Uh oh!

Lukshio commented Jun 25, 2026

Uh oh!

andreilakatos commented Jun 25, 2026

Uh oh!

andreilakatos commented Jun 25, 2026

Uh oh!

coderabbitai Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 25, 2026

Walkthrough

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Lukshio Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Lukshio commented Jun 30, 2026

Uh oh!

andreilakatos commented Jun 30, 2026

Uh oh!

Lukshio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Lukshio commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

andreilakatos commented May 22, 2026 •

edited

Loading

coderabbitai Bot commented Jun 25, 2026 •

edited

Loading