refactor(lineage): replace MUI Drawer and antd Modal with ui-core-components#29758
refactor(lineage): replace MUI Drawer and antd Modal with ui-core-components#29758chirag-madlani wants to merge 3 commits into
Conversation
…ponents LineageProvider.tsx was the last remaining MUI usage; swap the entity/edge side panel to SlideoutMenu and the delete-edge confirmation to Modal/Dialog, matching the SlideoutMenu pattern already used by OntologyEntityPanel. Update the two Playwright helpers that clicked the antd .ant-btn-primary class inside the confirmation modal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| await page | ||
| .locator('[data-testid="delete-edge-confirmation-modal"] .ant-btn-primary') | ||
| .locator( | ||
| '[data-testid="delete-edge-confirmation-modal"] [data-testid="confirm-button"]' | ||
| ) | ||
| .dispatchEvent('click'); |
There was a problem hiding this comment.
💡 Bug: dispatchEvent('click') may not trigger react-aria onPress
The new confirm button ([data-testid="confirm-button"]) is a Button from @openmetadata/ui-core-components, which is built on react-aria's usePress/onPress rather than a native DOM onClick. The Playwright helpers deleteEdge and removeColumnLineage still trigger it via .dispatchEvent('click'). A programmatically dispatched (untrusted) click event does not reliably fire react-aria's onPress, since press handling is driven by the pointer/keyboard interaction sequence. This can make these helpers silently fail to confirm the deletion, causing flaky or broken lineage e2e tests. Prefer Playwright's .click(), which performs a full trusted interaction that react-aria recognizes:
await page
.locator('[data-testid="delete-edge-confirmation-modal"] [data-testid="confirm-button"]')
.click();
(This is speculative on the exact react-aria version behavior — worth verifying the helpers still confirm deletion when running the affected specs.)
Use a real Playwright click instead of dispatching a synthetic click event so react-aria's onPress fires reliably.:
await page
.locator(
'[data-testid="delete-edge-confirmation-modal"] [data-testid="confirm-button"]'
)
.click();
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
Jest test CoverageUI tests summary
|
🟡 Playwright Results — all passed (15 flaky)✅ 4062 passed · ❌ 0 failed · 🟡 15 flaky · ⏭️ 21 skipped
🟡 15 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
… swap
SlideoutMenu applies its className prop to both the outer ModalOverlay
and the inner Modal, so passing "lineage-entity-panel" via className
duplicated it onto two elements — Playwright's `.locator('.lineage-entity-panel')`
then hit strict-mode violations (2 elements matched) across every
DataAssetLineage/LineageFilters spec that opens and closes the panel.
Move the selector-class onto dialogClassName, which only targets the
single Dialog element.
Also give ModalWithFunctionEditor (antd Modal opened from within the
lineage edge drawer) an explicit zIndex so its Save button always
stacks above the new SlideoutMenu overlay — it was being intercepted by
the panel's backdrop in LineageInteraction.spec.ts.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
| open={visible} | ||
| title={<Typography.Text data-testid="header">{header}</Typography.Text>} | ||
| width="700px" | ||
| zIndex={1050} |
There was a problem hiding this comment.
Any reason we need zindex ?
Since Toast is having zIndex of 1000, so confirm this thing once
There was a problem hiding this comment.
Good catch, and you're right to question it — I dug back in and there's no justification for it.
Verified concretely:
tw:z-999on the SlideoutMenu compiles toz-index: 999(checked via the Tailwind v4 compiler directly).- antd's default
.ant-modal-wrap/.ant-modal-maskz-index is1000(checked in the compiledantd.css).
So antd's modal already stacks above our panel by default (1000 > 999) — no override needed, and bumping to 1050 would've also incorrectly outranked Toast (also z-index 1000), exactly as you flagged.
The Save-button interception this was band-aiding was almost certainly a side effect of the actual bug: SlideoutMenu applies its className prop to both the outer ModalOverlay and inner Modal, so passing lineage-entity-panel via className duplicated that class onto two elements, which is what caused the bulk of the CI failures (Playwright strict-mode violations). I've since moved that class to dialogClassName (single element only) in a follow-up commit, and removed the zIndex override here — pushed in f49e20c.
…itor
Verified antd's default z-index for .ant-modal-wrap/.ant-modal-mask is
1000 (compiled antd.css) and our SlideoutMenu overlay's tw:z-999
compiles to z-index:999 (verified via the Tailwind v4 compiler), so
antd's modal already stacks above the lineage panel without an
override. The zIndex={1050} bump was an unjustified band-aid from the
earlier fix and would incorrectly outrank Toast notifications (also
z-index 1000). The Save-button interception seen in
LineageInteraction.spec.ts was almost certainly a symptom of the
duplicate lineage-entity-panel class fixed in the previous commit.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
| <Dialog | ||
| data-testid="delete-edge-confirmation-modal" | ||
| width={400} | ||
| onClose={() => setShowDeleteModal(false)}> |
There was a problem hiding this comment.
💡 Quality: Dialog onClose prop is dead code in delete-edge modal
The Dialog in the delete-edge confirmation passes onClose={() => setShowDeleteModal(false)}, but in the ui-core-components Dialog implementation onClose is only wired to the CloseButton, which is rendered exclusively when showCloseButton is true. Since showCloseButton is not set here, this onClose handler is never invoked — it is dead code. Actual dismissal is handled correctly by the parent ModalOverlay's onOpenChange and the footer's cancel/confirm Buttons. This is harmless but misleading, since it suggests the Dialog participates in close logic when it does not. Consider removing the prop to avoid confusion (dismissal is already fully handled by ModalOverlay and the footer buttons).
Remove the no-op onClose prop from Dialog; dismissal is handled by ModalOverlay's onOpenChange and the footer buttons.:
<Modal>
<Dialog
data-testid="delete-edge-confirmation-modal"
width={400}>
<Dialog.Header title={t('message.remove-lineage-edge')} />
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
Code Review 👍 Approved with suggestions 1 resolved / 3 findingsRefactors Lineage UI by replacing MUI and antd components with core-components primitives, while updating Playwright selectors to maintain test stability. Note that the new confirm button may require simulating pointer events instead of generic clicks, and ensure the Dialog's 💡 Bug: dispatchEvent('click') may not trigger react-aria onPress📄 openmetadata-ui/src/main/resources/ui/playwright/utils/lineage.ts:191-195 📄 openmetadata-ui/src/main/resources/ui/playwright/utils/lineage.ts:615-619 The new confirm button ( (This is speculative on the exact react-aria version behavior — worth verifying the helpers still confirm deletion when running the affected specs.) Use a real Playwright click instead of dispatching a synthetic click event so react-aria's onPress fires reliably.💡 Quality: Dialog onClose prop is dead code in delete-edge modal📄 openmetadata-ui/src/main/resources/ui/src/context/LineageProvider/LineageProvider.tsx:2228-2231 The Remove the no-op onClose prop from Dialog; dismissal is handled by ModalOverlay's onOpenChange and the footer buttons.✅ 1 resolved✅ Quality: Dialog onClose prop is a no-op (not part of react-aria Dialog API)
🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
Summary
LineageProvider.tsxwas the last remaining MUI (@mui/material) usage in the codebase; replaced itsDrawer(entity/edge side panel) withSlideoutMenufrom@openmetadata/ui-core-components.Modal(delete-lineage-edge confirmation) with the core-componentsModalOverlay/Modal/Dialogprimitives, preserving the loading/disabled button behavior viaButton'sisLoading/isDisabledprops.isDismissableto theSlideoutMenuto preserve the original Drawer's backdrop-click-to-close behavior (matches the existingOntologyEntityPanelpattern).deleteEdge,removeColumnLineageinplaywright/utils/lineage.ts) that clicked the antd.ant-btn-primaryclass inside the confirmation modal — that class no longer exists, so they now target[data-testid="confirm-button"].Test plan
tsc --noEmitclean (only a pre-existing, unrelated error inLineageProvider.test.tsxremains — confirmed present before this change viagit stash)LineageProvider.test.tsx— 11/11 passLineage.test.tsx,PlatformLineage.test.tsx,ContainerPage.test.tsx,LineageTable.test.tsx— all passEntityLineageJest suite — 18 suites / 249 tests passLineageRightPanel.spec.ts,LineageFilters.spec.ts,PlatformLineage.spec.ts,DataAssetLineage.spec.ts) — all use stabledata-testids preserved in the new markupui-checkstylesequence (organize-imports → eslint --fix → prettier) run on changed files — no residual diff🤖 Generated with Claude Code
Greptile Summary
This PR removes the last MUI
Drawerusage in the codebase by replacing it withSlideoutMenufrom@openmetadata/ui-core-components, and replaces the antdModaldelete-edge confirmation dialog with the core-componentsModalOverlay/Modal/Dialog/Buttonprimitives. Two Playwright helpers are updated to target the new[data-testid="confirm-button"]instead of the now-removed.ant-btn-primaryCSS class.LineageProvider.tsx: MUIDrawer→SlideoutMenu(preserving backdrop-click-to-close viaisDismissable); antdModal→ModalOverlay+DialogwithButton'sisLoading/isDisabledprops replacing thegetLoadingStatusValuehelper; the unusedgetLoadingStatusValueimport is removed.playwright/utils/lineage.ts:deleteEdgeandremoveColumnLineagenow target[data-testid="delete-edge-confirmation-modal"] [data-testid="confirm-button"]instead of.ant-btn-primary.Confidence Score: 5/5
Safe to merge — this is a pure component-library substitution with no logic changes, and all existing test IDs used by Playwright specs remain reachable in the new DOM structure.
Both replacements (SlideoutMenu for MUI Drawer, ModalOverlay/Dialog for antd Modal) are straightforward one-to-one substitutions that preserve open/close state, loading-disabled button behavior, and the z-index layering. The Playwright selector changes correctly target the new data-testid attributes. The getLoadingStatusValue import removal is clean. The lineage-entity-panel CSS class is now on the Dialog element rather than the old MUI Drawer root, matching how existing Playwright locators already target inner panel content.
No files require special attention.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant User participant LineageProvider participant SlideoutMenu participant ModalOverlay participant Dialog Note over LineageProvider: Node/Edge click User->>LineageProvider: click lineage node/edge LineageProvider->>SlideoutMenu: "isOpen=true" SlideoutMenu-->>User: slides in from right User->>SlideoutMenu: click backdrop or close button SlideoutMenu->>LineageProvider: onOpenChange(false) LineageProvider->>SlideoutMenu: "isOpen=false" SlideoutMenu-->>User: slides out Note over LineageProvider: Delete edge flow User->>LineageProvider: click delete edge LineageProvider->>ModalOverlay: "isOpen=true" ModalOverlay->>Dialog: render confirmation Dialog-->>User: Remove Lineage Edge dialog User->>Dialog: click Confirm Dialog->>LineageProvider: onPress to onRemove LineageProvider->>LineageProvider: "deletionState.loading=true" Dialog-->>User: Button isLoading and isDisabled LineageProvider->>ModalOverlay: setShowDeleteModal(false) ModalOverlay-->>User: dialog dismissed%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant User participant LineageProvider participant SlideoutMenu participant ModalOverlay participant Dialog Note over LineageProvider: Node/Edge click User->>LineageProvider: click lineage node/edge LineageProvider->>SlideoutMenu: "isOpen=true" SlideoutMenu-->>User: slides in from right User->>SlideoutMenu: click backdrop or close button SlideoutMenu->>LineageProvider: onOpenChange(false) LineageProvider->>SlideoutMenu: "isOpen=false" SlideoutMenu-->>User: slides out Note over LineageProvider: Delete edge flow User->>LineageProvider: click delete edge LineageProvider->>ModalOverlay: "isOpen=true" ModalOverlay->>Dialog: render confirmation Dialog-->>User: Remove Lineage Edge dialog User->>Dialog: click Confirm Dialog->>LineageProvider: onPress to onRemove LineageProvider->>LineageProvider: "deletionState.loading=true" Dialog-->>User: Button isLoading and isDisabled LineageProvider->>ModalOverlay: setShowDeleteModal(false) ModalOverlay-->>User: dialog dismissedReviews (3): Last reviewed commit: "fix(lineage): drop unjustified zIndex ov..." | Re-trigger Greptile