Fixes #38697 - ForemanForm in Webhook to PF5 #89

Lukshio · 2025-09-30T07:32:13Z

Replace ForemanForm with new PF5 variant

Copilot

Pull Request Overview

This pull request replaces the legacy ForemanForm with PatternFly 5 (PF5) components in the webhook test modal functionality, addressing issue #38697. The migration involves transitioning from Formik-based form handling to direct state management with PF5 form components.

Replaced ForemanForm and ForemanFormikField with PF5 Form, ActionGroup, and Button components
Introduced custom FieldConstructor and AutocompleteInput components for form field management
Updated form state management from Formik to React useState with manual validation

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
WebhookTestModal.js	Updated to use PF5 Form components and local state management
WebhookForm.js	Replaced ForemanForm with PF5 Form and custom validation logic
WebhookFormTabs.js	Updated to use FieldConstructor instead of ForemanFormikField
FieldConstructor.js	New component providing PF5-based form field rendering with validation
AutocompleteInput.js	New PF5 Select component for dropdown/typeahead functionality
ForemanFormikField.js	Removed legacy Formik-based field component
Test files	Updated from snapshot tests to React Testing Library with comprehensive coverage

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/FieldConstructor.js

MariaAga

Thanks, adding this as an initial review, with some comments as well
When editing a webhook the selected template is shown as an ID instead of a text value

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/__tests__/WebhookForm.test.js

...bhooks/Routes/Webhooks/Components/WebhookForm/Components/__tests__/AutocompleteInput.test.js

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/FieldConstructor.js

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/AutocompleteInput.js

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/FieldConstructor.js

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/AutocompleteInput.js

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/FieldConstructor.js

webpack/ForemanWebhooks/Routes/Webhooks/WebhooksIndexPage/Components/WebhookTestModal.js

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/FieldConstructor.js

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/WebhookFormTabs.css

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/AutocompleteInput.js

webpack/ForemanWebhooks/Routes/Webhooks/WebhooksIndexPage/Components/WebhookDeleteModal.js

Copilot

Pull Request Overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

webpack/ForemanWebhooks/Routes/Webhooks/WebhooksIndexPage/Components/WebhookTestModal.js

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/FieldConstructor.js

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/AutocompleteInput.js

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/WebhookFormTabs.js

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/AutocompleteInput.js

Lukshio · 2025-10-22T14:46:08Z

Adjust AutocompleteInput component to prepare move to foremanCore

MariaAga · 2025-11-13T09:55:31Z

I think the spacing issues between input and labels is because the <form doesnt wrap the form groups but the sections

Lukshio · 2025-11-13T20:49:40Z

Modal is now aligned to top with 100px offset.
Form field has extra 10px margin top
Pencil is showing only if field is disabled

kfamilonidis · 2025-11-14T11:05:53Z

@kfamilonidis When approving a PR could you please say what did you check? (the whole code, parts of it, tested it in development setup/broker, etc.)

Sure, my approval is to give priority to this as I see very robust test coverage to:

a) FieldConstructor.test.js‎
b) AutocompleteInput.js‎

bright useage of previous tests to:

a) WebhookFormTabs.js‎
b) WebhookForm/WebhookForm.js‎ introduction to FormField

Introduction to:

a) WebhooksTable/index.js‎
b) WebhookTestModal.js‎

Removal of:

a) ForemanFormikField.js‎
b) integration.test.js.snap‎

This PR is already very large. It's part of a broader effort to merge changes from the general upgrade process, and it may resolve many issues that are blocking other PRs.

MariaAga

The pencil icon only shows up when the same webhook is opened for editing for the second time (refresh didnt resolve this)

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/WebhookFormTabs.css

Lukshio · 2025-11-26T08:34:37Z

The pencil icon only shows up when the same webhook is opened for editing for the second time (refresh didnt resolve this)

I can't reproduce this issue.

adamruzicka · 2025-11-26T13:01:50Z

Modal is now aligned to top with 100px offset.
Pencil is showing only if field is disabled

Looks good, thank you.

Form field has extra 10px margin top

This has a downside of maybe too much padding for the first entry? If this could be solved easily then great, if not then I won't insist on it.

Could the checkbox be in line with its label?
-- #89 (comment)

This is still valid. In patternfly examples, the checkboxes always have an inline description. If needed, we could probably introduce a common header for the checkboxes and then have each of them have its own inline label. Alternatively we could go for a heading and on/off radios.

adamruzicka · 2025-11-26T13:07:45Z

The pencil icon only shows up when the same webhook is opened for editing for the second time (refresh didnt resolve this)

I can't reproduce this issue.

Fwiw, I can. Create a webhook, force refresh, click on that webhook. It is fine on subsequent opens, but not on the first one.

Lukshio · 2025-11-29T03:22:00Z

I found new issue related with the extra padding in combination with top offset. The action buttons are not visible on lower screen resolutions. So the solution is remove the offset and only align to top.

For the checkbox it's required, in the correct way, to set the form to horizontal layout, which can be beneficial here, because it will also solve the large height of Modal.

Finally I can reproduce the issue with password field. It's caused by the useState() rendering in new cycle so first open will not handle the state correctly.

Before making any changes, do you agree with changing to horizontal layout? @adamruzicka

adamruzicka · 2025-12-01T08:17:27Z

do you agree with changing to horizontal layout?

No objections against that

adamruzicka · 2025-12-08T15:38:26Z

Could we somehow disable browser's autocomplete for the typeahead fields?

adamruzicka · 2025-12-08T15:46:00Z

Same suggestion as in f-templates.

Could the info icons be prevented from spilling onto their own line?

Assuming it won't go against design guidelines or something, could the labels be right-aligned to make it appear less jagged?

adamruzicka

Only the pencil is required, the rest are more or less nice to haves.

adamruzicka · 2025-12-08T15:40:02Z

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/FieldConstructor.js

+          <GridItem span={1}>
+            <Button
+              ouiaId={`reset-${name}`}
+              onClick={() => setIsDisabled(false)}


This seems to be broken, when editing a webhook, I can't make the field become write-able

Lukshio · 2025-12-11T11:27:08Z

I added the extra gap, same as in templates. And with the labels, its the same situation, design guidlines only mentions top and left align
https://v5-archive.patternfly.org/components/forms/form/design-guidelines#labels

Password field should be fixed and autocomplete removed

kfamilonidis · 2025-12-16T11:28:12Z

Looks good to me. I tested it on packit/build. I see: labels correctly aligned, password properly adjusted, and pencil included. Noticed also about the icons in validation. It is handled ok for this specific case.

MariaAga

autocomplete=target_url causes the autocomplete to not work like it does in the old form
autocomplete=off for the HTTP Method causes it to auto fill username
Clicking on the pencil causes username and password autofill (which didnt happen in the old form)

MariaAga · 2025-12-16T11:54:15Z

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/FieldConstructor.js

+}) => (
+  <FormGroup
+    isInline
+    fieldId={fieldId}


I think filling up the fieldId should fix this issue:
26 No label associated with a form field
A isn't associated with a form field.

To fix this issue, nest the in the or provide a for attribute on the that matches a form field id.

pr-processor bot added the Not yet reviewed label Sep 30, 2025

Lukshio force-pushed the webhookFormUpdate branch 4 times, most recently from d47d95b to 82af696 Compare September 30, 2025 11:47

MariaAga requested a review from Copilot October 7, 2025 13:24

Copilot AI reviewed Oct 7, 2025

View reviewed changes

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/FieldConstructor.js Outdated Show resolved Hide resolved

pr-processor bot removed the Not yet reviewed label Oct 7, 2025

MariaAga self-assigned this Oct 7, 2025

MariaAga requested changes Oct 7, 2025

View reviewed changes

pr-processor bot added the Waiting on contributor label Oct 7, 2025

Lukshio force-pushed the webhookFormUpdate branch from 82af696 to 80bcec7 Compare October 9, 2025 20:01

pr-processor bot added Needs re-review and removed Waiting on contributor labels Oct 9, 2025

MariaAga requested changes Oct 10, 2025

View reviewed changes

pr-processor bot added Waiting on contributor and removed Needs re-review labels Oct 10, 2025

MariaAga requested a review from Copilot October 10, 2025 12:59

Copilot AI reviewed Oct 10, 2025

View reviewed changes

Lukshio force-pushed the webhookFormUpdate branch from 80bcec7 to 56f23fc Compare October 14, 2025 12:58

pr-processor bot added Needs re-review and removed Waiting on contributor labels Oct 14, 2025

Lukshio requested a review from MariaAga October 14, 2025 14:58

pr-processor bot removed the Needs re-review label Oct 15, 2025

adamruzicka reviewed Oct 16, 2025

View reviewed changes

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/AutocompleteInput.js Show resolved Hide resolved

MariaAga added the breaks-robottelo Can be set via comment: /label breaks-robottelo label Oct 16, 2025

Lukshio force-pushed the webhookFormUpdate branch from 56f23fc to 306c266 Compare October 22, 2025 14:45

This was referenced Oct 22, 2025

Fixes #38852 - Move new AutocompleteInput component to foreman core theforeman/foreman#10734

Open

Fixes #38853 - Change import of AutocompleteInput #91

Open

Lukshio force-pushed the webhookFormUpdate branch from 40a97b7 to 79500d7 Compare November 13, 2025 20:49

MariaAga requested changes Nov 21, 2025

View reviewed changes

webpack/ForemanWebhooks/Routes/Webhooks/Components/WebhookForm/Components/WebhookFormTabs.css Outdated Show resolved Hide resolved

pr-processor bot added the Waiting on contributor label Nov 21, 2025

Lukshio force-pushed the webhookFormUpdate branch from 79500d7 to 1accc52 Compare November 26, 2025 09:04

pr-processor bot added Needs re-review and removed Waiting on contributor labels Nov 26, 2025

Lukshio force-pushed the webhookFormUpdate branch from 1accc52 to 51a4a00 Compare December 8, 2025 15:13

adamruzicka requested changes Dec 8, 2025

View reviewed changes

pr-processor bot added Waiting on contributor and removed Needs re-review labels Dec 8, 2025

Fixes #38697 - ForemanForm in Webhook to PF5

4146a5c

Lukshio force-pushed the webhookFormUpdate branch from 51a4a00 to 4146a5c Compare December 11, 2025 11:31

pr-processor bot added Needs re-review and removed Waiting on contributor labels Dec 11, 2025

MariaAga requested changes Dec 16, 2025

View reviewed changes

pr-processor bot added Waiting on contributor and removed Needs re-review labels Dec 16, 2025

MariaAga reviewed Dec 16, 2025

View reviewed changes

Fixes #38697 - ForemanForm in Webhook to PF5 #89

Are you sure you want to change the base?

Fixes #38697 - ForemanForm in Webhook to PF5 #89

Uh oh!

Conversation

Lukshio commented Sep 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

MariaAga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Lukshio commented Oct 22, 2025

Uh oh!

MariaAga commented Nov 13, 2025

Uh oh!

Lukshio commented Nov 13, 2025

Uh oh!

kfamilonidis commented Nov 14, 2025

Uh oh!

MariaAga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Lukshio commented Nov 26, 2025

Uh oh!

adamruzicka commented Nov 26, 2025

Uh oh!

adamruzicka commented Nov 26, 2025

Uh oh!

Lukshio commented Nov 29, 2025

Uh oh!

adamruzicka commented Dec 1, 2025

Uh oh!

adamruzicka commented Dec 8, 2025

Uh oh!

adamruzicka commented Dec 8, 2025

Uh oh!

adamruzicka left a comment

Choose a reason for hiding this comment

Uh oh!

adamruzicka Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Lukshio commented Dec 11, 2025

Uh oh!

kfamilonidis commented Dec 16, 2025

Uh oh!

MariaAga left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MariaAga left a comment •

edited

Loading

MariaAga Dec 16, 2025 •

edited

Loading