bug: Dialog overlay overlaps Combobox

[yb-cs]

Current Behavior

When I add "<Command.Group class="overflow-y-auto h-40">" to the combobox to make it scrollable it causes the text to become blurry and low resolution. However, in the case when it does not scroll the text is fine.

Expected Behavior

Same text style when the area is not scrollable.

Steps To Reproduce

1. Use combobox
2. Use options large enough to scroll
3. Notice the blur / degraded text
4. Please check the image attatched.
   

Link to Reproduction / Stackblitz (reproduction template)

No response

More Information

No response

[yb-cs]

Forgot to mention it was used inside a dialog. It could be the blur styling leaking in? I am not sure...

[huntabyte]

If you increase the z-index to something like 50 of the Content does the issue persist?

[yb-cs]

This is interesting I added the following "<Command.Group class="overflow-y-auto h-40 z-50">" initially it is not blurry but once you scroll it becomes blurry again.

[yb-cs]

See the difference before and after scrolling.

[huntabyte]

Add it to the Command.List

[yb-cs]

it is actually worse than group. Somehow group fixed it before scrolling but this is blurry in both cases:


<Command.List class="overflow-y-auto h-40 z-50">
<Command.Group>

and

    <Command.List>
      <Command.Group>

[yb-cs]

I figured out the issue. It is 100% because using it with dialog. Probably a css class contamination or something of that sort since if I do it on a normal page it is not blury, however the dialog has the blur effect and it is causing this.

[yb-cs]

How would you advice on this? is it because in a list they are not in focus and it causes the blur effect to bleed in?

[yb-cs]

@huntabyte could you advice how to proceed further with this? should I avoid using the dialog with combobox or is there a way to prevent the style bleeding for the blur effect?

Thanks in advance :)

[huntabyte]

Related:
shadcn-ui/ui#2363
shadcn-ui/ui#2638

[slinso]

I looked at a few different Dialog implementations and it seems others are using (IMO) a simpler approach to the centering/placing of the dialog.

Instead of:

<Dialog.Portal>
    <Dialog.Overlay />
    <DialogPrimitive.Content class="fixed translate..." />
</Dialog.Portal>

use this and get rid of the translate css:

<Dialog.Portal>
    <Dialog.Overlay class="fixed flex justify-center items-center>
        <DialogPrimitive.Content />
    </Dialog.Overlay>
</Dialog.Portal>

I tried the simplest solution and just added a slot to the Dialog.Overlay, but melt/bits does not work with with nested markup. I think a change in bits-ui could make that work.

@huntabyte
What do you think? Is it worth investing more time into this approach?

[huntabyte]

@huntabyte What do you think? Is it worth investing more time into this approach?

I believe this would mess with how transitions happen since the overlay and content should kick off their transitions at the same time. This feels more like a CSS styling issue than anything else tbh.

[GimpMaster]

Not sure if it helps much but I found out that in web browsers the blurryness in selects is present.

However in my capacitor app running on Safari it’s gone. Kind of strange.

[slinso]

Ok, it is some kind of CSS issue, but not really a simple one to solve. If you look around stackedoverflow there are a few questions regarding this issue (e.g. SO: Blurry Text

Here is a good explanation and a possible fix: GPU Text Rendering in Webkit. So for dialogs which have the maximum width we could just change "sm:max-w[425px]" to something even like "sm:max-w[426px]".

But my main concern is, that this problem is only introduced by using the transform function translate to center the dialog. I tried playing around with anti-aliasing and other approaches I found on stackedoverflow, but the problem still persists. I could get a hacky solution working on my machine, but rendering on a different machine/screen size/windowed mode still raises this issue.

So I took a look a the melt-ui examples und bits-ui code. I exaggerated the transition effect in all examples. In melt-ui the fix is pretty simple. Change

<div use:melt={$overlay} 
  class="fixed inset-0 z-50 bg-black/50" 
  transition:fade={{ duration: 500 }} />
<div
	class="fixed left-1/2 top-1/2 -translate-x-1/2 -translate-y-1/2
		z-50 max-h-[85vh] w-[90vw] max-w-[450px] rounded-xl bg-white p-6 shadow-lg"
	transition:flyAndScale={{ duration: 1000, y: 40, start: 0.96 }}
	use:melt={$content}>
	...
</div>

to

<div use:melt={$overlay} 
  class="fixed inset-0 z-50 bg-black/50 flex items-center justify-center" 
  transition:fade={{ duration: 500 }}>
		<div
			class="z-50 max-h-[85vh] w-[90vw] max-w-[450px] rounded-xl bg-white p-6 shadow-lg"
			transition:flyAndScale={{ duration: 1000, y: 40, start: 0.96 }}
			use:melt={$content}>
			...
		</div>
</div>

The transitions will be triggered at the same time, because they are not nested in different components. So for bits-ui I had to make to Dialog.Content transition global and I added a default slot to every if branch other than the "asChild" branch. Like this:

dialog-content.svelte:

		transition:transition|global={transitionConfig}

dialog-overlay.svelte:

	<div on:mouseup	bind:this={el} transition:transition={transitionConfig}	use:melt={builder} {...$$restProps}>
		<slot />
	</div>

Then I used the code with the nested Dialog.Content from my
#534 (comment) to use flex for centering the dialog.

This issue is not really shadcn/svelte specific, but rather a CSS issue that is present the dialog examples for melt-ui, which was then reused in the other 2 projects.
IMO it is best to get rid of the transform functions.

I could write proper pull requests for each project, if you want me to.
The question would just be if this fix should be applied to all 3 projects?

Related:
huntabyte/cmdk-sv#54

[slinso]

I found a really old (2018) Ember issue regarding this topic. The solution is always the same: use flex (or grid) layout to solve rendering bugs (or work around rendering behavior).

@huntabyte, if you do not want to apply the change in all three projects, would you be fine with a pull request for bits-ui, so I can apply my fix locally? Without that fix, I would need to maintain a copy of bits-ui, which is obviously not my intention.

[david-plugge]

Is there a workaround that i can use today until this gets fixed?

[slinso]

Hey @david-plugge I just submitted the PR for the necessary changes to bits-ui. You can of course use my fork, but I would advise against that, as I do not plan to keep the fork up to date. I hope the PR gets merged.

You would have to make these additional changes to your local shadcn code:

dialog-content.svelte:

• nest the DialogPrimitive.Content inside Dialog.Overlay
• remove "fixed left-[50%] top-[50%] translate-x-[50%] translate-y-[50%]" from DialogPrimitive.Content

dialog-overlay.svelte:

• add "flex items-center justify-center"
• add to DialogPrimitive.Overlay

[david-plugge]

My workaround looks like this now:

<script lang="ts">
	import { Dialog as DialogPrimitive, builderActions, getAttrs } from 'bits-ui';
	import * as Dialog from '.';
	import { cn, flyAndScale } from '$lib/utils';
	import { X } from 'lucide-svelte';

	type $$Props = DialogPrimitive.ContentProps;

	let className: $$Props['class'] = undefined;
	export let transition: $$Props['transition'] = flyAndScale;
	export let transitionConfig: $$Props['transitionConfig'] = {
		duration: 200,
	};
	export { className as class };

	$: _transition = transition || flyAndScale;
</script>

<Dialog.Portal>
	<Dialog.Overlay />

	<DialogPrimitive.Content asChild let:builder>
		<div class="fixed inset-0 z-50 grid place-items-center">
			<div
				transition:_transition={transitionConfig}
				use:builderActions={{ builders: [builder] }}
				{...getAttrs([builder])}
				{...$$restProps}
				class={cn(
					'grid w-full max-w-lg gap-4 border bg-background p-6 shadow-lg sm:rounded-lg md:w-full',
					className,
				)}
			>
				<slot />

				<DialogPrimitive.Close
					class="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-accent data-[state=open]:text-muted-foreground"
				>
					<X class="h-4 w-4" />
					<span class="sr-only">Close</span>
				</DialogPrimitive.Close>
			</div>
		</div>
	</DialogPrimitive.Content>
</Dialog.Portal>

[GimpMaster]

@david-plugge - Does your work around require changes to bits-ui like @slinso mentioned?

[slinso]

@david-plugge clickOutside to close the dialog is not working for me. Otherwise a nice solution as well. I haven't looked at what is missing. I'll invest a little more time into my approach.

[david-plugge]

@GimpMaster no, you only need to change dialog-content.svelte from shadcn-svelte
@slinso I updated my example so it works with clicking outside.

[Dart353]

I made some adjustments to @david-plugge solution to align it with our code style. However, when I added the border-radius to <DialogPrimitive.Content />, I noticed that it caused blurring issues with the select options.

To resolve this, I used <DialogPrimitive.Content /> as a wrapper to handle positioning. Then, I added a single child <div> inside it and passed the props, slots, and styling to it - just like @david-plugge did.

This approach solved the blurring issues with the select.

My code:

<Dialog.Portal>
	<Dialog.Overlay />
	<DialogPrimitive.Content
		{transition}
		{transitionConfig}
		class={cn(
			small ? 'max-w-[343px]' : 'max-w-[690px]',
			'fixed inset-0 left-[50%] top-[50%] z-50 grid h-fit max-h-[80%] w-full translate-x-[-50%] translate-y-[-50%] place-items-center ',
			className
		)}
	>
		<div
			{...$$restProps}
			class={cn(
				'bg-background shadow-lg md:w-full grid w-full max-w-lg gap-4 rounded-16 bg-background-surface p-16',
				className
			)}
		>
			<slot />
		</div>
	</DialogPrimitive.Content>
</Dialog.Portal>

[slinso]

Thanks again @david-plugge for your workaround. I had some issues with my nested-dialog-content approach, so I switched to your approach and it just works. I closed my pull request and want to leave this topic behind.

@Dart353
You are still using the translate CSS functions, which david removed in his code. As long, as you are using translate... a user of your app may have blurry text. It just depends on the actual pixel sizes at which the dialog is rendered.

[wesharper]

I've searched around and I think I found the root cause.

The TL;DR is that the font is not aligned with the pixel grid, causing webkit to render it strangely when antialiasing. Translating in either x or y direction can cause this misalignment depending on the size of the thing being translated.

My solution is similar to those above, and uses flex for centering. It only requires editing the shad-cn files and touches no other libraries.

1. Render DialogPrimitive.Overlay as a child div that centers its children and allows for passing of a slot.
2. Wrap DialogPrimitive.Content in the new DialogOverlay.
3. Add 'relative' class to DialogPrimitive.Content to ensure the close button gets put in the right spot.
4. Update transitions for dialog-overlay.

dialog-content.svelte

<script lang="ts">
  import { Dialog as DialogPrimitive } from 'bits-ui';
  import Cross2 from 'svelte-radix/Cross2.svelte';
  import * as Dialog from './index.js';
  import { cn, flyAndScale } from '$lib/utils.js';

  type $$Props = DialogPrimitive.ContentProps;

  let className: $$Props['class'] = undefined;
  export let transition: $$Props['transition'] = flyAndScale;
  export let transitionConfig: $$Props['transitionConfig'] = {
    duration: 200
  };
  export { className as class };
</script>

<Dialog.Portal>
  <Dialog.Overlay>
    <DialogPrimitive.Content
      class={cn(
        'relative grid w-[90%] gap-4 rounded-lg border bg-background p-6 shadow-lg sm:max-w-lg',
        className
      )}
      {transition}
      {transitionConfig}
      {...$$restProps}
    >
      <slot />
      <DialogPrimitive.Close
        class="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-accent data-[state=open]:text-muted-foreground"
      >
        <Cross2 class="h-4 w-4" />
        <span class="sr-only">Close</span>
      </DialogPrimitive.Close>
    </DialogPrimitive.Content>
  </Dialog.Overlay>
</Dialog.Portal>

dialog-overlay.svelte

<script lang="ts">
  import { Dialog as DialogPrimitive } from 'bits-ui';
  import { fade } from 'svelte/transition';
  import { cn } from '$lib/utils.js';

  type $$Props = DialogPrimitive.OverlayProps;

  let className: $$Props['class'] = undefined;
  export let transitionConfig: $$Props['transitionConfig'] = {
    duration: 150
  };
  export { className as class };
</script>

<DialogPrimitive.Overlay asChild let:builder>
  <div
    use:builder.action
    transition:fade={transitionConfig}
    {...$$restProps}
    class={cn(
      'fixed inset-0 z-50 flex items-center justify-center bg-background/80 backdrop-blur-sm ',
      className
    )}
    {...builder}
  >
    <slot />
  </div>
</DialogPrimitive.Overlay>

One possible improvement could be modifying bits so that the overlay accepts a slot, which could then be passed down without having to delegate rendering of DialogPrimitive.Overlay to the child, which would require less manual work for custom transitions.