Fixes #36106: PF5 Refactor - EditorNavbar, EditorOptions

[kfamilonidis]

Background

Editor and <EditorNavBar /> upgrade to next version of react requires substitution of Enzyme tests with RTL (React Testing Library) framework.

Acceptance Criteria

• Upgrade of components to use PF/5 variants.
  • Hosts -> Templates -> Job Templates -> New Job Template (path: /job_templates/new)
  • Hosts -> Templates -> Job Templates -> (any or clone) -> Edit (path: /job_templates/xyz/edit)
• Removal of Enzyme in all related tests.

Tests

npm run test -- assets/javascripts/react_app/components/Editor assets/javascripts/react_app/components/DiffView

followed by: #10777

[kfamilonidis]

Still needs progress to remove the snaps from the test that are testing components properties, but have noticed that some snaps are different from others; some are keeping store props, and some keeping components props. I would like to ask for some opinion on should we remove both snap types, or should we keep the store props - as the later might be serve as fixtures in the future (maybe transition to other store).

[kfamilonidis]

I would like to ask for some opinion on should we remove both snap types, or should we keep the store props - as the later might be serve as fixtures in the future (maybe transition to other store).

The *Actions, *Selectors and *Reducer snapshots are not replaceable with the RTL (React Testing Library) approach. They test logic that falls outside of RTL's context. Similarly, the integration tests snapshots. Importantly, these snapshots are not blocking the major React version upgrade

[kfamilonidis]

I removed the snaps related to integration.test.js and redux-store.

[kfamilonidis]

this is needed as react-ace is 3rd party library and we don't need/test to call the original.

[MariaAga]

Tested it on a dev env and it works as it should, left some comments and I still need to go over the new tests and test with packit

[MariaAga]

Why the word changes?

[kfamilonidis]

no reason actually, I think I was testing the length of the message but, is handled perfectly fine with the previous copy. Reverted it.

[MariaAga]

With these changes the safemode text is not aligned with the checkbox, could you fix it here or open a follow up task for it? maybe converting it to PF from html will help

[kfamilonidis]

I tested it with PF5 Label and the result is a bit strange, pls see:

When opening the select host select menu, renders like here
. This menu is updated here or with autocomplete here
I could center the element with the custom css that is already there here leaving the upgrade to happen with set host menu or pls advice.

[MariaAga]

The check boxkes here are also not aligned (like safemode), I checked and its because we have a global rule to set label bottom-margin to 5px. I think we should have a global rule to set bottom-margin to 0 for pf forms/checkboxes.
context: The rule comes from pf3

[MariaAga]

Should be more specific as other components use nav-tabs-pf

[kfamilonidis]

this rule is overriding a previous global issue with a css rule that caused the padding of the first element to be 0.

[Copilot]

Pull request overview

This PR successfully migrates the Editor components (EditorNavbar, EditorOptions, EditorSettings, EditorRadioButton) from PatternFly 4 to PatternFly 5, and replaces all Enzyme tests with React Testing Library (RTL) tests.

Key changes:

• Upgraded PF components to PF5 variants (Button, Nav, Alert, Spinner, FormSelect, Checkbox)
• Replaced Enzyme snapshot tests with RTL assertion-based tests
• Updated CSS class names from nav-tabs-pf-secondary to nav-tabs-pf5-secondary

Reviewed changes

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

Show a summary per file

File	Description
editor.scss	Updated CSS classes for PF5 compatibility and removed unused popover styles
EditorSettings.js	Migrated from PF3 Dropdown to PF5 FormSelect and Checkbox components
EditorRadioButton.js	Updated to use PF5 NavItem with isActive prop
EditorOptions.js	Migrated Button components to use PF5 variant and isDisabled props
EditorNavbar.js	Updated Nav, Alert, and Spinner components to PF5 variants
EditorSettings.test.js	Replaced Enzyme snapshots with RTL tests validating form interactions
EditorRadioButton.test.js	Replaced Enzyme with RTL, testing component rendering
EditorOptions.test.js	Migrated from Enzyme mount/simulate to RTL render/fireEvent
EditorNavbar.test.js	Updated to use RTL with proper role-based queries
EditorModal.test.js	Converted to RTL with document query selectors
EditorHostSelect.test.js	Migrated to RTL with comprehensive DOM assertions
EditorView.test.js	Replaced Enzyme snapshots with RTL tests and mocked react-ace
Editor.test.js	Full RTL migration with proper role queries and assertions
integration.test.js	Updated to use RTL helpers and removed snapshot-based testing
DiffView.test.js	Migrated from snapshot tests to RTL with table role assertions
Diff.fixtures.js	Updated fixture keys to simple names instead of descriptions

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The disabled prop should be updated to isDisabled to match PatternFly 5 API for the NavItem component.

Suggested change

	disabled={disabled}
	isDisabled={disabled}

[kfamilonidis]

this is breaking the app in tests.

[Copilot]

The disabled prop should be updated to isDisabled to match PatternFly 5 API. Line 124 still uses the old disabled prop instead of isDisabled.

Suggested change

	disabled={selectedView !== 'input'}
	isDisabled={selectedView !== 'input'}

[MariaAga]

Also seeing how much css overrides we do to make the Nav component behave like Tab component, and with the new pf5 css I noticed also the active tab title are too bright so they will also need an override for the colour, why not just use the tab component?