CONSOLE-5012: Migrate confirmModal to overlay pattern by rhamilto · Pull Request #15948 · openshift/console

rhamilto · 2026-01-23T20:58:04Z

Summary

This PR completes the modal migration effort by removing the deprecated confirmModal (which used createModalLauncher) and migrating to the modern overlay pattern with an imperative launchWarningModal() API.

Changes

Removed deprecated confirmModal

✅ Deleted confirm-modal.tsx (72 lines removed)
✅ Removed confirmModal export from public/components/modals/index.ts
✅ Eliminated createModalLauncher pattern usage
✅ Removed unused i18n key "Confirm" from public.json

Created imperative warning modal API

✅ Created warning-modal-handler.tsx with launchWarningModal() function
✅ Follows same pattern as launchErrorModal()
✅ Promise-based API: resolves on confirm, rejects with Error('User cancelled') on cancel
✅ Works from non-React contexts (drag-drop callbacks, utility functions)
✅ Uses modern overlay system via useOverlay hook
✅ Race condition guards: Only clears launcher if still active
✅ Comprehensive JSDoc documentation with examples
✅ Warning icon is optional via titleIconVariant prop (defaults to 'warning' per PatternFly)

Unified modal launcher initialization with race condition protection

✅ Refactored SyncErrorModalLauncher → SyncModalLaunchers in error-modal-handler.tsx
✅ Single component syncs both error and warning modal launchers
✅ Added reference-based cleanup guards to prevent race conditions
✅ Mounted once in app.tsx after OverlayProvider

Migrated moveNodeToGroup to use launchWarningModal

✅ Updated moveNodeToGroup.tsx to use launchWarningModal() instead of confirmModal()
✅ Preserved <Trans> components with  tags for bold text formatting
✅ Added onError parameter for custom error handling
✅ Integrated with launchErrorModal() for error display
✅ Proper error handling distinguishing user cancellation from actual errors
✅ Hides warning icon for move/remove operations via titleIconVariant: null

Fixed accessibility violation

✅ Blur active element before launching confirmation modal in moveNodeToGroup.tsx:753-761
✅ Prevents focus from remaining on SVG elements in topology graph
✅ Fixes browser warning: "Blocked aria-hidden on an element because its descendant retained focus"

Fixed useWarningModal type safety

✅ Exported ControlledWarningModal component from useWarningModal.tsx for reuse
✅ Uses proper OverlayComponent type instead of manual prop definition
✅ Properly calls closeOverlay() on modal dismiss

Updated metal3-plugin modals

✅ Converted stopNodeMaintenanceModal to useStopNodeMaintenanceModal hook in StopNodeMaintenanceModal.tsx
✅ Added optional nodeMaintenance type support (K8sResourceKind | undefined)
✅ Updated StartingMaintenancePopoverContent.tsx to use hook
✅ Updated UnderMaintenancePopoverContent.tsx to use hook
✅ Updated maintenance actions.tsx with proper hook dependencies

Updated shipwright-plugin

✅ Cleaned up orphaned functions in logs-utils.ts
✅ Removed getOrderedStepsFromPod (no references found - 20 lines removed)
✅ Removed getDownloadAllLogsCallback (no references found - 111 lines removed)
✅ Removed unused i18n key "Error downloading logs."

Test infrastructure

✅ Created comprehensive test suite in warning-modal-handler.spec.tsx (6 test cases)
✅ Created mocks/warning-modal-handler.tsx for Jest tests
✅ Updated mocks/error-modal-handler.tsx to export SyncModalLaunchers
✅ Updated error-modal-handler.spec.tsx to test unified component
✅ Changed from console.error to throw Error when launcher not initialized
✅ Documented mock limitations regarding circular import concerns

Architecture Benefits

Consistent pattern - Both error and warning modals use the same imperative launcher pattern
Simpler than React Context - No provider/consumer boilerplate needed
Single initialization point - One SyncModalLaunchers component for all imperative modals
Works everywhere - Usable from both React components and non-React contexts
Type-safe - Throws clear errors when launchers not initialized
Race condition protection - Reference-based cleanup prevents clearing newer launchers
Proper cleanup - Via React lifecycle, not manual cleanup
WCAG 2.1 Level A compliant - No aria-hidden focus violations
Well-tested - Comprehensive unit tests for all scenarios
Well-documented - Clear JSDoc with examples and proper error types
Flexible icon display - Warning icon can be hidden when not needed

Technical Details

Optional Warning Icon

The ControlledWarningModal wrapper now passes through the titleIconVariant prop, allowing callers to:

Use default warning icon - Don't specify titleIconVariant (PatternFly defaults to 'warning')
Hide the icon - Pass titleIconVariant: null
Use a different icon - Pass titleIconVariant: 'danger' | 'success' | 'info' or a custom icon

Example usage in moveNodeToGroup.tsx:

launchWarningModal({
  title: i18next.t(titleKey),
  children: message,
  confirmButtonLabel: i18next.t(btnTextKey),
  titleIconVariant: null, // Hides the warning icon
})

Race Condition Protection

Problem: If components mount/unmount out of order, the cleanup could clear a newer launcher.

Solution: Reference-based cleanup in both launchers (error-modal-handler.tsx:194-205, warning-modal-handler.tsx:278-289):

useEffect(() => {
  const errorModalLauncher = (props: ErrorModalProps) => {
    launcher<ErrorModalProps>(ErrorModal, props);
  };
  moduleErrorModalLauncher = errorModalLauncher;
  
  return () => {
    // Only clear if we're still the active launcher
    if (moduleErrorModalLauncher === errorModalLauncher) {
      moduleErrorModalLauncher = null;
    }
  };
}, [launcher]);

This prevents race conditions where unmounting could clear a newer launcher that was set by a remounting component.

Error Handling Improvements

Promise Rejection with Proper Error Objects:

// Before: rejected with undefined
onClose: () => {
  reject();
}

// After: rejects with Error object
onClose: () => {
  reject(new Error('User cancelled'));
}

This allows downstream code to properly detect user cancellation:

.catch((error) => {
  // Only show error modal if it's not a user cancellation
  if (error.message && error.message !== 'User cancelled') {
    launchErrorModal({ error: error.message });
  }
});

Type Safety for Optional nodeMaintenance

Problem: useStopNodeMaintenanceModal was called with potentially undefined nodeMaintenance but didn't handle it.

Solution: Updated signature and added guards (StopNodeMaintenanceModal.tsx:509-526):

export const useStopNodeMaintenanceModal = (nodeMaintenance: K8sResourceKind | undefined) => {
  const reason = nodeMaintenance ? getNodeMaintenanceReason(nodeMaintenance) : '';
  const nodeName = nodeMaintenance ? getNodeMaintenanceNodeName(nodeMaintenance) : '';
  // ...
  onConfirm: nodeMaintenance
    ? () => k8sKill(getMaintenanceModel(nodeMaintenance), nodeMaintenance)
    : () => Promise.resolve(),
}

Comprehensive Documentation

JSDoc with Clear Type Information:

/**
 * Launch a warning modal from non-React contexts (callbacks, promises, utilities).
 * 
 * @param props - Warning modal properties (title, children, confirmButtonLabel, etc.)
 * @returns Promise that resolves (undefined) when confirmed, rejects with Error('User cancelled') when canceled/closed
 * 
 * @example
 * ```tsx
 * launchWarningModal({
 *   title: 'Delete Resource',
 *   children: 'Are you sure you want to delete this resource?',
 *   confirmButtonLabel: 'Delete',
 * }).then(() => {
 *   // User confirmed - proceed with action
 *   deleteResource();
 * }).catch((error) => {
 *   // User canceled - error.message === 'User cancelled'
 *   console.log('Action cancelled by user');
 * });
 * ```
 */

Test Coverage

6 Comprehensive Test Cases (warning-modal-handler.spec.tsx):

✅ SyncModalLaunchers - should sync the warning launcher on mount
✅ SyncModalLaunchers - should cleanup warning launcher on unmount
✅ useWarningModalLauncher - should return a function that launches warning modals
✅ launchWarningModal - should launch warning modal when launcher is initialized
✅ launchWarningModal - should throw error when launcher is not initialized
✅ launchWarningModal - should resolve promise when onConfirm is called
✅ launchWarningModal - should reject promise with Error when onClose is called

Test Patterns:

Follows same structure as error-modal-handler.spec.tsx
Uses React Testing Library
Mocks useOverlay for isolation
Tests both sync and async behaviors
Validates error handling and race condition scenarios

Accessibility Fix - aria-hidden Focus Violation

Problem: When the confirmation modal opened during drag-drop operations, focus remained on an SVG <g> element in the topology graph. PatternFly's Modal sets aria-hidden="true" on the #app div, creating an accessibility violation where a focused element is hidden from assistive technology users.

Browser Warning (Fixed):

Blocked aria-hidden on an element because its descendant retained focus.
The focus must not be hidden from assistive technology users.
Element with focus: <g>
Ancestor with aria-hidden: <div#app>

Solution: Blur the active element before launching the modal (moveNodeToGroup.tsx:753-761):

// Blur active element to prevent aria-hidden focus violations when modal opens
// This fixes the browser warning: "Blocked aria-hidden on an element because its
// descendant retained focus" when focus remains on SVG elements during drag-drop
if (
  document.activeElement instanceof HTMLElement ||
  document.activeElement instanceof SVGElement
) {
  document.activeElement.blur();
}

Imperative Warning Modal Pattern

Before (deprecated createModalLauncher):

// ❌ Old pattern using createModalLauncher
confirmModal({
  titleKey,
  message,
  btnTextKey,
  close: () => reject(),
  cancel: () => reject(),
  executeFn: () => updateTopologyResourceApplication(...),
});

After (modern overlay pattern):

// ✅ New pattern using launchWarningModal with Trans for formatting
const message = targetGroup ? (
  <Trans ns="topology">
    Are you sure you want to move <strong>{{ nodeLabel }}</strong> from {{ sourceLabel }} to {{ targetLabel }}?
  </Trans>
) : (
  <Trans ns="topology">
    Are you sure you want to remove <strong>{{ nodeLabel }}</strong> from {{ sourceLabel }}?
  </Trans>
);

launchWarningModal({
  title: i18next.t(titleKey),
  children: message,
  confirmButtonLabel: i18next.t(btnTextKey),
  titleIconVariant: null, // Hide the warning icon for simple confirmation
})
  .then(() => updateTopologyResourceApplication(...))
  .catch((err) => {
    if (err.message !== 'User cancelled') {
      launchErrorModal({ error: err.message });
    }
  });

Key Improvements:

Promise-based instead of callback-based
Uses modern overlay system
Consistent with launchErrorModal() pattern
No deprecated createModalLauncher usage
Preserves <Trans> for formatted text with bold styling
Proper error type discrimination
Optional icon display

Single SyncModalLaunchers Component

// Before: Single error launcher
<OverlayProvider>
  <SyncErrorModalLauncher />
  <YourApp />
</OverlayProvider>

// After: Unified component for both error and warning
<OverlayProvider>
  <SyncModalLaunchers />
  <YourApp />
</OverlayProvider>

Implementation (error-modal-handler.tsx:224-227):

export const SyncModalLaunchers = () => {
  useSyncErrorModalLauncher();
  useSyncWarningModalLauncher();
  return null;
};

Mock Architecture & Circular Import Handling

Documentation in Mock (mocks/error-modal-handler.tsx):

The mock provides a simplified SyncModalLaunchers that doesn't call useSyncWarningModalLauncher. This is intentional and acceptable:

Mock Purpose: Simplifies tests that don't need real modal rendering
Explicit Dependencies: Tests needing warning modals should explicitly mock or import
No Circular Import: Import chain is unidirectional (error → warning)
Test Isolation: Each test suite mocks only what it needs

Why This Design Works:

Runtime code has clear, unidirectional dependencies
Mocks document their limitations
Tests are explicit about their needs
Follows Jest best practices

i18n Translation Updates

The <Trans> components with  tags are properly detected by the i18n tooling:

// Before
"Are you sure you want to move {{nodeLabel}} from {{sourceLabel}} to {{targetLabel}}?"

// After
"Are you sure you want to move <1>{{nodeLabel}}</1> from {{sourceLabel}} to {{targetLabel}}?"

The <1> placeholder represents the  tag, ensuring translated text maintains proper formatting.

Test Plan

Automated Tests ✅

TypeScript compilation succeeds
ESLint passes
Build succeeds
i18n keys updated correctly
Unit tests pass (error-modal-handler.spec.tsx: 5/5)
New test suite created (warning-modal-handler.spec.tsx: 6 test cases)

Manual Testing Checklist

Breaking Changes

None - this is a refactoring that maintains the same functionality.

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.

openshift-ci-robot · 2026-01-23T21:02:19Z

@rhamilto: This pull request references CONSOLE-5012 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.22.0" version, but no target version was set.

Details

In 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.

coderabbitai · 2026-01-23T21:08:20Z

📝 Walkthrough

Walkthrough

This pull request introduces a modal infrastructure refactor across the OpenShift Console codebase, shifting from a direct modal launcher pattern (via createModalLauncher) to an overlay-based provider pattern with React lazy loading. The changes include: introducing OverlayComponent-based modal providers wrapping existing modal components in ModalWrapper; implementing useOverlay() hooks to obtain launchModal functions for programmatic modal invocation; consolidating error handling through a new global error modal handler utility; creating lazy-loaded modal exports for code-splitting; removing launchModal from dependency arrays with ESLint suppressions to address potential max-depth errors; and updating action creators and modal invocation sites across frontend packages to use the new launchModal(ProviderComponent, props) pattern instead of direct function calls.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title 'CONSOLE-5012: Migrate confirmModal to overlay pattern' accurately summarizes the primary change—removing the legacy confirmModal launcher and migrating it to the overlay/useWarningModal pattern.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ 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.}

coderabbitai

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

🤖 Fix all issues with AI agents

In
`@frontend/packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx`:
- Around line 279-283: The spread props currently come after explicit close
props in ClonePVCModalProvider which allows props.closeOverlay to be overridden;
update the JSX so {...props} is spread first and then pass
cancel={props.closeOverlay} and close={props.closeOverlay} (affecting the
ClonePVCModal inside ModalWrapper) to ensure closeOverlay always wins and cannot
be overridden by incoming props.

In
`@frontend/packages/console-app/src/components/modals/restore-pvc/restore-pvc-modal.tsx`:
- Around line 282-287: The provider currently spreads props after passing
cancel/close to RestorePVCModal, allowing a caller to override those with their
own cancel/close; change RestorePVCModalProvider so you destructure closeOverlay
from props first (e.g., const { closeOverlay, ...rest } = props) then render
<RestorePVCModal cancel={closeOverlay} close={closeOverlay} {...rest} /> inside
ModalWrapper, ensuring props.closeOverlay cannot be overridden by
caller-supplied props.

In
`@frontend/packages/console-app/src/components/nodes/modals/ConfigureUnschedulableModal.tsx`:
- Around line 51-64: The provider currently spreads props after assigning cancel
and close which allows caller props to override them; update
ConfigureUnschedulableModalProvider to destructure props into { closeOverlay,
...rest } (or extract known keys) and then render ConfigureUnschedulableModal
with {...rest} followed by explicit cancel={closeOverlay} and
close={closeOverlay} so the overlay's close handlers cannot be overridden;
ensure ModalWrapper still uses closeOverlay for onClose and pass the same
closeOverlay to ConfigureUnschedulableModal.

In `@frontend/packages/console-shared/src/utils/error-modal-handler.ts`:
- Around line 35-50: The cleanup unconditionally clears the global launcher
causing newer launchers to be removed; fix useSetupGlobalErrorModalLauncher by
capturing the specific errorModalLauncher reference you set via
setGlobalErrorModalLauncher and, in the return cleanup, only call
setGlobalErrorModalLauncher(null) if the current global launcher equals that
captured reference (so use a stable local/ref for errorModalLauncher and guard
the cleanup), referencing the existing functions errorModalLauncher,
useSetupGlobalErrorModalLauncher, and setGlobalErrorModalLauncher to locate and
implement the guarded cleanup.

In `@frontend/packages/knative-plugin/src/actions/creators.ts`:
- Around line 4-5: The SDK currently forces imports from an internal path to
reach useOverlay; update the SDK public exports so useOverlay can be imported
from '@console/dynamic-plugin-sdk' by re-exporting the modal overlay API: add an
export for the modal-support module (e.g., export * from './modal-support') from
the SDK app index or alternatively export the dynamic-core-api module from the
SDK root so that the symbol useOverlay is publicly available; modify the module
that aggregates app exports to re-export useOverlay (and related modal types) so
consumers can import { useOverlay } from '@console/dynamic-plugin-sdk'.

In `@frontend/public/components/modals/configure-update-strategy-modal.tsx`:
- Around line 241-250: The provider currently spreads {...props} after setting
cancel/close so a caller can override cancel/close; update
ConfigureUpdateStrategyModalProvider to spread props first into
ConfigureUpdateStrategyModal (or omit cancel/close from the spread) and then
pass cancel={props.closeOverlay} and close={props.closeOverlay} to enforce using
props.closeOverlay; locate the ConfigureUpdateStrategyModalProvider and
ConfigureUpdateStrategyModal usages to apply this change so closeOverlay cannot
be overridden by caller props.

In `@frontend/public/components/modals/delete-modal.tsx`:
- Around line 183-188: The DeleteModalProvider currently spreads props after
passing cancel and close, allowing a caller to override cancel/close; in the
DeleteModalProvider component (and the ModalWrapper usage) spread {...props}
first and then pass cancel={props.closeOverlay} and close={props.closeOverlay}
so the enforced overlay handlers cannot be overridden by caller-provided props
(refer to DeleteModalProvider, ModalWrapper, DeleteModal, and
props.closeOverlay).

In `@frontend/public/components/modals/delete-pvc-modal.tsx`:
- Around line 87-91: The DeletePVCModalProvider currently spreads {...props}
after explicit cancel/close props which allows callers to override
props.closeOverlay; change the prop order in the JSX so you spread {...props}
first and then pass cancel={props.closeOverlay} close={props.closeOverlay} (on
ModalWrapper keep onClose={props.closeOverlay}) to ensure DeletePVCModal and
ModalWrapper always use props.closeOverlay for teardown and cannot be overridden
by incoming props.

♻️ Duplicate comments (2)

🧹 Nitpick comments (16)

frontend/packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx

frontend/packages/console-app/src/components/modals/restore-pvc/restore-pvc-modal.tsx

frontend/packages/console-app/src/components/nodes/modals/ConfigureUnschedulableModal.tsx

frontend/packages/console-shared/src/utils/error-modal-handler.ts

frontend/packages/knative-plugin/src/actions/creators.ts

frontend/public/components/modals/configure-update-strategy-modal.tsx

frontend/public/components/modals/delete-modal.tsx

frontend/public/components/modals/delete-pvc-modal.tsx

rhamilto · 2026-01-24T13:24:23Z

/retest

rhamilto · 2026-01-25T02:16:27Z

/retest

rhamilto · 2026-01-25T16:49:01Z

/retest

rhamilto · 2026-01-28T13:09:57Z

/retest

rhamilto · 2026-01-28T14:54:35Z

/retest

rhamilto · 2026-01-29T00:04:10Z

/retest

rhamilto · 2026-02-02T17:17:39Z

/assign @yanpzhan
/assign @vojtechszocs

Tech debt, so assigning labels
/label px-approved
/label docs-approved

openshift-ci-robot · 2026-02-18T14:54:54Z

@rhamilto: This pull request references CONSOLE-5012 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.22.0" version, but no target version was set.

Details

In 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.

openshift-ci-robot · 2026-02-18T15:06:30Z

@rhamilto: This pull request references CONSOLE-5012 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.22.0" version, but no target version was set.

Details

In 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.

openshift-ci-robot · 2026-02-18T15:07:35Z

@rhamilto: This pull request references CONSOLE-5012 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.22.0" version, but no target version was set.

Details

In 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.

openshift-ci-robot · 2026-02-18T15:11:02Z

@rhamilto: This pull request references CONSOLE-5012 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.22.0" version, but no target version was set.

Details

In 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.

rhamilto · 2026-02-18T15:13:08Z

@logonoff, please take another look. Ended up refactoring so we only have a single SyncModalLaunchers component for both error and confirm modals.

openshift-ci-robot · 2026-02-18T15:30:46Z

@rhamilto: This pull request references CONSOLE-5012 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.22.0" version, but no target version was set.

Details

In 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.

frontend/packages/console-shared/src/utils/error-modal-handler.tsx

openshift-ci-robot · 2026-02-18T16:08:23Z

@rhamilto: This pull request references CONSOLE-5012 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.22.0" version, but no target version was set.

Details

In 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.

openshift-ci-robot · 2026-02-18T16:13:45Z

@rhamilto: This pull request references CONSOLE-5012 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.22.0" version, but no target version was set.

Details

In 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.

openshift-ci-robot · 2026-02-18T16:33:49Z

@rhamilto: This pull request references CONSOLE-5012 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.22.0" version, but no target version was set.

Details

In 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.

rhamilto · 2026-02-18T18:03:56Z

/hold

Just realized there are cosmetic differences in the modal since I repurposed warning.

- Add error-modal-handler.tsx to console-shared package with: - SyncErrorModalLauncher - Zero-render component that syncs launcher for non-React contexts - useErrorModalLauncher() - Hook for launching error modals from React components - launchErrorModal() - Function for launching from non-React contexts (callbacks, promises) - Add SyncErrorModalLauncher to app.tsx inside OverlayProvider - Single initialization point for the entire application - Reuses existing OverlayProvider instead of creating new context - Migrate packages to use launchErrorModal: - topology: componentUtils.ts, edgeActions.ts - knative-plugin: knativeComponentUtils.ts, create-connector-utils.ts - shipwright-plugin: logs-utils.ts - dev-console: pipeline-template-utils.ts - Update moveNodeToGroup.tsx to support error handling: - Add setMoveNodeToGroupErrorHandler() for managing error handler - Add useSetupMoveNodeToGroupErrorHandler() hook for Topology component - Update moveNodeToGroup() to accept optional onError callback - Deprecate useMoveNodeToGroup() hook - Add comprehensive test coverage: - error-modal-handler.spec.tsx - 5 unit tests covering all scenarios - __mocks__/errorModalHandler.ts - Global mock for testing - Fixed useWarningModal to properly call closeOverlay() on modal close/confirm Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

openshift-ci-robot · 2026-02-18T18:40:07Z

@rhamilto: This pull request references CONSOLE-5012 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.22.0" version, but no target version was set.

Details

In 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.

rhamilto · 2026-02-18T18:41:21Z

/hold cancel

Removed the warning icon from the confirm modal title so we match what we had before

Screen.Recording.2026-02-18.at.1.38.52.PM.mov

rhamilto · 2026-02-19T13:24:33Z

/retest

rhamilto · 2026-02-19T20:06:35Z

/retest

openshift-ci · 2026-02-20T03:50:30Z

@rhamilto: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-gcp-console	`1132d49`	link	true	`/test e2e-gcp-console`

Full PR test history. Your PR dashboard.

Details

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.

openshift-merge-robot · 2026-02-20T03:50:37Z

PR needs rebase.

Details

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.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 23, 2026

rhamilto changed the title ~~CONSOLE-5012: Migrate confirmModal to overlay pattern~~ [WIP] CONSOLE-5012: Migrate confirmModal to overlay pattern Jan 23, 2026

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 23, 2026

openshift-ci bot requested review from jhadvig and sg00dwin January 23, 2026 20:59

coderabbitai bot reviewed Jan 23, 2026

View reviewed changes

rhamilto force-pushed the CONSOLE-5012-5 branch 3 times, most recently from d102d05 to 274b735 Compare January 27, 2026 20:39

rhamilto force-pushed the CONSOLE-5012-5 branch from 274b735 to 73300e0 Compare January 28, 2026 16:27

openshift-ci bot assigned vojtechszocs Feb 2, 2026

rhamilto force-pushed the CONSOLE-5012-5 branch 3 times, most recently from cfb2db3 to 6ba05ee Compare February 18, 2026 15:03

logonoff reviewed Feb 18, 2026

View reviewed changes

frontend/packages/console-shared/src/utils/error-modal-handler.tsx Outdated Show resolved Hide resolved

rhamilto force-pushed the CONSOLE-5012-5 branch from 6ba05ee to daef88e Compare February 18, 2026 16:06

rhamilto force-pushed the CONSOLE-5012-5 branch from daef88e to 854a75a Compare February 18, 2026 16:12

rhamilto force-pushed the CONSOLE-5012-5 branch 2 times, most recently from 4481a7b to eb9b49f Compare February 18, 2026 16:25

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 18, 2026

rhamilto force-pushed the CONSOLE-5012-5 branch from eb9b49f to 1132d49 Compare February 18, 2026 18:37

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 18, 2026

rhamilto force-pushed the CONSOLE-5012-5 branch from 29d12cd to 1132d49 Compare February 18, 2026 18:45

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 20, 2026

Comments

Conversation

rhamilto commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Removed deprecated confirmModal

Created imperative warning modal API

Unified modal launcher initialization with race condition protection

Migrated moveNodeToGroup to use launchWarningModal

Fixed accessibility violation

Fixed useWarningModal type safety

Updated metal3-plugin modals

Updated shipwright-plugin

Test infrastructure

Architecture Benefits

Technical Details

Optional Warning Icon

Race Condition Protection

Error Handling Improvements

Type Safety for Optional nodeMaintenance

Comprehensive Documentation

Test Coverage

Accessibility Fix - aria-hidden Focus Violation

Imperative Warning Modal Pattern

Single SyncModalLaunchers Component

Mock Architecture & Circular Import Handling

i18n Translation Updates

Test Plan

Automated Tests ✅

Manual Testing Checklist

Breaking Changes

Related

Uh oh!

openshift-ci-robot commented Jan 23, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Technical Details

Topology Move Node Migration

StopNodeMaintenanceModal Migration

useWarningModal Fix

Dependencies

Test Plan

Uh oh!

openshift-ci-robot commented Jan 23, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Technical Details

Topology Move Node Migration

StopNodeMaintenanceModal Migration

useWarningModal Fix

Dependencies

Test Plan

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jan 23, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

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!

Uh oh!

Uh oh!

Uh oh!

rhamilto commented Jan 24, 2026

Uh oh!

rhamilto commented Jan 25, 2026

Uh oh!

rhamilto commented Jan 25, 2026

Uh oh!

rhamilto commented Jan 28, 2026

Uh oh!

rhamilto commented Jan 23, 2026 •

edited

Loading

openshift-ci-robot commented Jan 23, 2026 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Jan 23, 2026 •

edited by openshift-ci bot

Loading

coderabbitai bot commented Jan 23, 2026 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Feb 18, 2026 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Feb 18, 2026 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Feb 18, 2026 •

edited by openshift-ci bot

Loading