Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Putting dialog trigger inside dropdown menu adds an extra div #902

Closed
ChrisPoul opened this issue Mar 13, 2024 · 5 comments
Closed

Putting dialog trigger inside dropdown menu adds an extra div #902

ChrisPoul opened this issue Mar 13, 2024 · 5 comments
Labels
status: needs reproduction A reproduction is required to support the issue.

Comments

@ChrisPoul
Copy link

Describe the bug

I wan't to use a dialog and put it's trigger inside a dropdown menu, the issue I'm having is that it messes up the look since it adds an extra div that wasn't there before.

Here's how it looks when not using the dropdown component:
image

It behaves as expected with no problem, but when I instead try to use the dropdown menu this happens:
image

Now there is sudenly an extra div right after the trigger button and this messes up the look of the links.

Here's the code of the dropdown menu:

<script lang="ts">
	import NavLinks from './NavLinks.svelte';
	import { Button } from '$lib/components/ui/button/index.js';
	import * as DropdownMenu from '$lib/components/ui/dropdown-menu/index.js';
</script>

<DropdownMenu.Root>
	<DropdownMenu.Trigger asChild let:builder>
		<Button builders={[builder]} variant="outline">Open</Button>
	</DropdownMenu.Trigger>
	<DropdownMenu.Content
		class="p-6 2xs:px-10 text-center flex flex-col gap-4 bg-black bg-opacity-90"
	>
		<NavLinks />
	</DropdownMenu.Content>
</DropdownMenu.Root>

And this is is the code in the NavLinks component:

<a class="link" href="/#nosotros" rel="ugc"> Nosotros </a>
<a class="link" href="/#servicios" rel="ugc"> Servicios </a>
<a class="link" href="/soporte" target="_blank" rel="ugc"> Soporte </a>
<a class="link" href="/#clientes" rel="ugc"> Clientes </a>
<Contact>
	<button class="link">Contacto</button>
</Contact>
<a class="link" href="/tienda" rel="ugc"> Tienda en Línea</a>

<style lang="postcss">
	.link {
		@apply font-medium text-black hover:text-gray-700
		dark:text-white dark:hover:text-zinc-300 hover:scale-105;
	}
</style>

It's just a couple of a tags, I need to do it this way because I use those tags somewhere else with a different format.

Reproduction

You can just copy and paste the code I provided in the description

Logs

No response

System Info

System:
    OS: Linux 5.15 openSUSE Tumbleweed 20240305
    CPU: (12) x64 AMD Ryzen 5 Microsoft Surface (R) Edition
    Memory: 3.73 GB / 7.49 GB
    Container: Yes
    Shell: 3.6.0 - /usr/bin/fish
  Binaries:
    Node: 21.7.1 - ~/.local/share/nvm/v21.7.1/bin/node
    Yarn: 1.22.19 - /mnt/c/Users/Chris/AppData/Roaming/npm/yarn
    npm: 10.5.0 - ~/.local/share/nvm/v21.7.1/bin/npm
    pnpm: 8.15.4 - ~/.local/share/pnpm/pnpm
  npmPackages:
    @sveltejs/kit: ^2.0.0 => 2.5.2
    bits-ui: ^0.19.5 => 0.19.5
    svelte: ^4.2.7 => 4.2.12

Severity

annoyance

@huntabyte huntabyte added the status: needs reproduction A reproduction is required to support the issue. label Mar 15, 2024
Copy link
Contributor

Please provide a reproduction.

More info

Why do I need to provide a reproduction?

This project is maintained by a very small team, and we simply don't have the bandwidth to investigate issues that we can't easily replicate. Reproductions enable us to fix issues faster and more efficiently. If you care about getting your issue resolved, providing a reproduction is the best way to do that.

I've provided a reproduction - what happens now?

Once a reproduction is provided, we'll remove the needs reproduction label and review the issue to determine how to resolve it. If we can confirm it's a bug, we'll label it as such and prioritize it based on its severity.

If needs reproduction labeled issues don't receive any activity (e.g., a comment with a reproduction link), they'll be closed. Feel free to comment with a reproduction at any time and the issue will be reopened.

How can I create a reproduction?

You can use this template to create a minimal reproduction. You can also link to a GitHub repository with the reproduction.

Please ensure that the reproduction is as minimal as possible. If there is a ton of custom logic in your reproduction, it is difficult to determine if the issue is with your code or with the library. The more minimal the reproduction, the more likely it is that we'll be able to assist.

You might also find these other articles interesting and/or helpful:

@gregmsanderson
Copy link

gregmsanderson commented Mar 18, 2024

Hi @ChrisPoul

I also ran into a problem when calling a Dialog from within a Dropdown menu. It's mentioned a lot on the original Shadcn repo (the ui.shadcn.com) and people suggest different ideas over there that you can probably translate here. This uses the same basic ideas after all.

It sounds like your issue may be solved by what I did: open the Dialog programatically using a standard Dropdown.Item element. That way the Dialog is not "in" the Dropdown menu. It's a sibling of it. So it won't add any div etc. which may be breaking your layout.

The idea is essentially this (in my case, it's an AlertDialog rather than a Dialog, but the same idea applies).

<DropdownMenu.Root>
	<DropdownMenu.Trigger asChild let:builder>
		<Button variant="ghost" builders={[builder]} class="relative h-8 w-8 p-0">
			<span class="sr-only">Open menu</span>
			<Icons.DotsHorizontal class="h-4 w-4" />
		</Button>
	</DropdownMenu.Trigger>
	<DropdownMenu.Content>
		<DropdownMenu.Item href="/example">Example</DropdownMenu.Item>
		<DropdownMenu.Item on:click={openDialog(exampleParam)}>Open dialog</DropdownMenu.Item>
	</DropdownMenu.Content>
</DropdownMenu.Root>

Notice that rather than have a Dialog with a Trigger component, there is an on:click instead. Now the Dialog does not need a Trigger component. So you can put the Dialog as a sibling, below it, and so its HTML can't affect the Dropdown HTML e.g

<AlertDialog.Root open={dialogOpen} onOpenChange={(open) => (dialogOpen = open)}>
	<AlertDialog.Content>
		<AlertDialog.Header>
			<AlertDialog.Title>Example</AlertDialog.Title>
			<AlertDialog.Description>Are you sure?</AlertDialog.Description>
		</AlertDialog.Header>
		<AlertDialog.Footer>
			<AlertDialog.Cancel>No</AlertDialog.Cancel>
		</AlertDialog.Footer>
	</AlertDialog.Content>
</AlertDialog.Root>

Note that since the Dialog is now being opened with an on:click you will need to keep track of its opened state with a boolean. Why? Well ... if someone opens the Dialog with a click on the Dropdown ... but then closes it by pressing Escape ... your UI is then out of sync. Hence the dialogOpen which is set as true within openDialog(...).

@ChrisPoul
Copy link
Author

Thank you for your help @gregmsanderson, this solved my issue

@Stadly
Copy link
Contributor

Stadly commented Oct 21, 2024

Very helpful, @gregmsanderson!

I think you could do <AlertDialog.Root bind:open={dialogOpen}> and let the two-way binding take care of keeping dialogOpen in sync :)

@gregmsanderson
Copy link

@Stadly No problem. You may well be right about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs reproduction A reproduction is required to support the issue.
Projects
None yet
Development

No branches or pull requests

4 participants