base-ui icon indicating copy to clipboard operation
base-ui copied to clipboard

[dialog][alert dialog] Add `renderMode` prop to `Backdrop` part

Open atomiks opened this issue 6 months ago • 9 comments

Closes #1477

atomiks avatar Jun 02 '25 06:06 atomiks

vite-css-base-ui-example

npm i https://pkg.pr.new/@base-ui-components/react@2037
npm i https://pkg.pr.new/@base-ui-components/utils@2037

commit: b44bc3f

pkg-pr-new[bot] avatar Jun 02 '25 06:06 pkg-pr-new[bot]

Deploy Preview for base-ui ready!

Name Link
Latest commit b44bc3fbab871c6d3aebe8fae576c2951853a2bd
Latest deploy log https://app.netlify.com/projects/base-ui/deploys/6874d5cad7996a0008492990
Deploy Preview https://deploy-preview-2037--base-ui.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

netlify[bot] avatar Jun 02 '25 06:06 netlify[bot]

Bundle size report

Bundle Parsed Size Gzip Size
@base-ui-components/react 🔺+40B(+0.01%) 🔺+21B(+0.02%)

Details of bundle changes

Generated by :no_entry_sign: dangerJS against b44bc3fbab871c6d3aebe8fae576c2951853a2bd

mui-bot avatar Jun 02 '25 07:06 mui-bot

Not a huge fan of the prop name. I saw quite a lot of renderXXX props that were render props (renderItem={item => <Item item={item} />}). So I wouldn't use the same type of name for a prop like the one created in this PR.

flaviendelangle avatar Jun 02 '25 12:06 flaviendelangle

@flaviendelangle yeah I don't like it either but it describes whether it doesn't render or not, so what would it be called?

atomiks avatar Jun 02 '25 12:06 atomiks

I don't have a very good proposal :/ nestedDialogRenderMode is very verbose

flaviendelangle avatar Jun 03 '25 09:06 flaviendelangle

nestedRenderMode seems ok though 🤔 @colmtuite

atomiks avatar Jun 04 '25 02:06 atomiks

Why would you want leaf instead of root or always?

colmtuite avatar Jun 04 '25 05:06 colmtuite

You want leaf when you don't want stacking effects and always want the topmost dialog to dim everything behind it - the linked issue shows a screenshot:

#1477

The stacked dialogs in the docs do dim a parent nested dialog already, but use an ::after overlay instead:

Screenshot 2025-06-04 at 3 55 49 pm

It turns out that due to re-parenting, leaf doesn't play nicely with transitions. Even if it animated here, the cross-fade may not be perfect with the regular ease timing function? So maybe the ::after pseudo-element is actually the best technique to dim parent dialogs already 🤔

https://github.com/user-attachments/assets/7b6f04b1-2f3c-4d38-b2b1-efcbb7c6643b

atomiks avatar Jun 04 '25 05:06 atomiks

As you mentioned in my feature request; I agree that a boolean should suffice for the use cases.

Would be nice if this gets merged :)

marcojacobsNL avatar Jul 09 '25 10:07 marcojacobsNL

This is currently preventing us from using the library for our UI. When do you think it could get a review? @colmtuite @michaldudak

marcojacobsNL avatar Jul 14 '25 09:07 marcojacobsNL

@marcojacobsNL you can use the canary pre-releases before it's released https://github.com/mui/base-ui/pull/2037#issuecomment-2929112103

atomiks avatar Jul 14 '25 10:07 atomiks

@michaldudak practically I don't think that works: https://jakearchibald.com/2021/dom-cross-fade/. I also think it's complex enough that it won't be used, as the ::after technique already used on the docs is significantly simpler to handle

atomiks avatar Jul 15 '25 05:07 atomiks