feat: migrate drawer components from MUI to SlideoutMenu in untitled UI#26518
feat: migrate drawer components from MUI to SlideoutMenu in untitled UI#26518
Conversation
openmetadata-ui/src/main/resources/ui/src/components/common/atoms/drawer/useDrawer.tsx
Outdated
Show resolved
Hide resolved
openmetadata-ui/src/main/resources/ui/src/components/common/atoms/drawer/useDrawerBody.tsx
Show resolved
Hide resolved
openmetadata-ui/src/main/resources/ui/src/components/common/atoms/drawer/useDrawer.tsx
Outdated
Show resolved
Hide resolved
🟡 Playwright Results — all passed (30 flaky)✅ 3323 passed · ❌ 0 failed · 🟡 30 flaky · ⏭️ 183 skipped
🟡 30 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 |
openmetadata-ui-core-components/src/main/resources/ui/src/components/foundations/typography.tsx
Show resolved
Hide resolved
|
@gitar-bot do an in depth frontend review of this refactor |
Here is the in-depth frontend review of this MUI → SlideoutMenu drawer refactor. In-Depth Frontend ReviewCritical Bugs1.
onClick: () => { // ← sync function
try {
setIsSubmitting(true);
onSubmit({} as T); // ← Promise returned but NOT awaited
} catch {
// this catch will never fire for rejected promises
} finally {
setIsSubmitting(false); // fires immediately, before API call completes
}
}For any async onClick: async () => {
try {
setIsSubmitting(true);
await onSubmit({} as T);
} catch {
// errors handled by caller
} finally {
setIsSubmitting(false);
}
}2. Overlay loader does not actually overlay — children remain visible during loading
<SlideoutMenu.Content className={cx('tw:relative', className)}>
{loading && (
<div className="tw:absolute tw:inset-0 tw:z-10 tw:flex tw:items-center tw:justify-center tw:bg-primary/60">
<Loader />
</div>
)}
{children}
</SlideoutMenu.Content>3. Loading overlay color is
Design/Architecture Issues4.
const compositeDrawer = useMemo(() => {
const DrawerComponent = baseDrawer.drawer;
const Component = DrawerComponent.type; // extracts Menu function
return <Component {...DrawerComponent.props}>{drawerContent}</Component>;
}, [baseDrawer.drawer, drawerContent]);This pattern manually reconstructs a React element by pulling const compositeDrawer = useMemo(
() => React.cloneElement(baseDrawer.drawer, {}, drawerContent),
[baseDrawer.drawer, drawerContent]
);Or better yet, simply pass children through 5.
useEffect(() => {
if (open) {
assetDrawer.openDrawer();
} else {
assetDrawer.closeDrawer();
}
}, [open, assetDrawer]); // ← assetDrawer is a new object every render
const { openDrawer, closeDrawer } = assetDrawer;
useEffect(() => {
if (open) openDrawer();
else closeDrawer();
}, [open, openDrawer, closeDrawer]);6.
isDismissable={config.closeOnBackdrop !== false}
isKeyboardDismissDisabled={config.closeOnEscape === false}These map correctly in isolation, but note: when Performance Issues7. Buttons in
key={button.testId ?? button.label}8.
API/TypeScript Issues9. Multiple callers previously passed 10.
11.
Accessibility12.
<SlideoutMenu.Header onClose={showCloseButton ? onClose : undefined}>This is what the code does — but since // In slideout-menu.tsx Header
{onClose !== undefined && (
<CloseButton size="md" className="..." onClick={onClose} />
)}13.
Summary Table
|
Code Review ✅ Approved 5 resolved / 5 findingsMigrates drawer components from MUI to SlideoutMenu in the untitled UI library, resolving five issues including dynamic Tailwind class generation, loading state overlay behavior, anchor configuration handling, async form submission, and typography defaults. All findings have been addressed. ✅ 5 resolved✅ Bug: Dynamic Tailwind classes won't generate CSS at build time
✅ Bug: Loading state no longer overlays content in drawer body
✅ Quality: DrawerConfig.anchor is declared but silently ignored
✅ Bug: Removing
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
|



Describe your changes:
Fixes
I worked on ... because ...
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>Summary by Gitar
DrawerwithSlideoutMenufrom@openmetadata/ui-core-componentsin drawer hooksanchor,height, andzIndexconfiguration options (SlideoutMenu defaults to right anchor)useDrawerwithhandleOpenChangecallback replacing MUI'sonClosehandleruseDrawerBodyto useSlideoutMenu.Contentwith Tailwind classesuseDrawerFooteranduseDrawerHeaderto use SlideoutMenu subcomponentsDomainsListPage.constants.tsand associatedDRAWER_HEADER_STYLINGreferencesprimary,secondary,tertiary)This will update automatically on new commits.