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 6 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}
Jest test CoverageUI tests summary
|
🟡 Playwright Results — all passed (23 flaky)✅ 4087 passed · ❌ 0 failed · 🟡 23 flaky · ⏭️ 22 skipped
🟡 23 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>
…se prop
- playwright/utils/lineage.ts: use a real .click() instead of
.dispatchEvent('click') for the confirm-button — it's a react-aria
Button (usePress/onPress), and a synthetic dispatched click isn't
guaranteed to trigger onPress the way a trusted Playwright click does.
Matches the .click() convention already used for this same testid
elsewhere in the suite (glossary.ts, testCases.ts, etc.).
- LineageProvider.tsx: drop the onClose prop passed to Dialog in the
delete-edge confirmation — ui-core-components' Dialog only wires
onClose to its CloseButton, which never renders here since
showCloseButton isn't set, so it was dead code. Dismissal is already
handled by ModalOverlay's onOpenChange and the footer buttons.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Now that MUI is gone, nothing else pulls in reactflow's base/style CSS transitively — import them directly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…omponents # Conflicts: # openmetadata-ui/src/main/resources/ui/src/styles/index.ts
Code Review ✅ Approved 3 resolved / 3 findingsReplaces the last MUI Drawer and antd Modal in the codebase with ui-core-components primitives to unify the UI design language. Playwright tests were updated to use stable data-testids, and all previous modal and drawer functionality remains preserved. ✅ 3 resolved✅ Quality: Dialog onClose prop is a no-op (not part of react-aria Dialog API)
✅ Bug: dispatchEvent('click') may not trigger react-aria onPress
✅ Quality: Dialog onClose prop is dead code in delete-edge modal
OptionsDisplay: 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/materialusage in the codebase by replacing the MUIDrawer(entity/edge side panel) withSlideoutMenufrom@openmetadata/ui-core-components, and replaces the antdModal(delete-edge confirmation) with the core-componentsModalOverlay/Modal/Dialogprimitives. Two Playwright helpers are updated to use stable[data-testid="confirm-button"]instead of the now-absent.ant-btn-primaryclass.SlideoutMenuis correctly driven byModalOverlay/Modal/Dialogfromslideout-menus/slideout-menu.tsx, while the delete confirmation modal correctly uses the separate centered-modal components frommodals/modal.tsx— both sourced through@openmetadata/ui-core-components's index.getLoadingStatusValueutility import is cleanly removed since button loading state is now expressed throughButton'sisLoading/isDisabledprops.dispatchEvent('click')(raw DOM event, no actionability checks) to.click()(proper Playwright interaction), which is a correctness improvement in addition to the selector fix.Confidence Score: 5/5
Safe to merge — the component swaps are mechanically correct, the delete modal correctly uses centered-modal primitives, and the Playwright helpers now target stable testids.
Both component replacements resolve to the right implementations via the package's index (centered modal vs slideout panel are separate exports). Existing Playwright tests that query children of
[data-testid="lineage-entity-panel"]remain valid because the testid moves from the MUI Drawer wrapper to the React Aria ModalOverlay wrapper — both are the outermost container element and all child elements remain nested within. No behavioral regressions were identified.No files require special attention.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant User participant LineageProvider participant SlideoutMenu participant ModalOverlay as ModalOverlay (centered) participant onRemove User->>LineageProvider: click lineage node/edge LineageProvider->>SlideoutMenu: "isOpen=true (entity/edge panel)" SlideoutMenu-->>User: slides in from right User->>SlideoutMenu: click backdrop (isDismissable) SlideoutMenu->>LineageProvider: onOpenChange(false) LineageProvider->>SlideoutMenu: "isOpen=false" SlideoutMenu-->>User: slides out User->>LineageProvider: click delete edge LineageProvider->>ModalOverlay: "isOpen=true (delete confirmation)" ModalOverlay-->>User: centered modal with Confirm / Cancel User->>ModalOverlay: "click Confirm (isLoading=true)" ModalOverlay->>onRemove: invoke onRemove-->>LineageProvider: success - setShowDeleteModal(false) LineageProvider->>ModalOverlay: "isOpen=false" ModalOverlay-->>User: modal 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 as ModalOverlay (centered) participant onRemove User->>LineageProvider: click lineage node/edge LineageProvider->>SlideoutMenu: "isOpen=true (entity/edge panel)" SlideoutMenu-->>User: slides in from right User->>SlideoutMenu: click backdrop (isDismissable) SlideoutMenu->>LineageProvider: onOpenChange(false) LineageProvider->>SlideoutMenu: "isOpen=false" SlideoutMenu-->>User: slides out User->>LineageProvider: click delete edge LineageProvider->>ModalOverlay: "isOpen=true (delete confirmation)" ModalOverlay-->>User: centered modal with Confirm / Cancel User->>ModalOverlay: "click Confirm (isLoading=true)" ModalOverlay->>onRemove: invoke onRemove-->>LineageProvider: success - setShowDeleteModal(false) LineageProvider->>ModalOverlay: "isOpen=false" ModalOverlay-->>User: modal dismissedReviews (5): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile