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

Use ShadCN's DropdownMenu in the Cloud UI #3737

Merged
merged 18 commits into from
Dec 21, 2023

Conversation

ericpgreen2
Copy link
Contributor

@ericpgreen2 ericpgreen2 commented Dec 20, 2023

This PR follows #3736, which introduces the ShadCN DropdownMenu component set. This PR uses the new dropdown menu in the following places in the Cloud UI:

  • The top bar's breadcrumbs' dropdown menus
  • The one-element menu that opens upon clicking a dashboard's "Share" button
  • The user avatar's dropdown menu in the top-right-hand corner of the application
  • The scheduled report page's dropdown (which shows the "Edit report", "Delete report" actions)

Closes #3630

Note: In the user avatar menu, I'd prefer if the "View As" submenu opened on click rather than on hover. I think I'm blocked by a bug with controlled dropdown menus. I've opened up the following issue in ShadCN-Svelte: huntabyte/bits-ui#245

Copy link
Contributor

@bcolloran bcolloran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, digging it.

import { createPopperActions } from "svelte-popperjs";
DropdownMenu,
DropdownMenuContent,
DropdownMenuItem,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nit, but I think import * as DropdownMenu and then accessing Content, Root, Trigger, etc. keeps this a lot cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and just updated. I was relying on VSCode auto-imports, so I'm curious – would you do this import * by hand, or do you happen to have a VSCode trick?

Copy link
Contributor

@briangregoryholmes briangregoryholmes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really great and solves a ton of little problems in one fell swoop. Added some comments about small implementation details.

@briangregoryholmes
Copy link
Contributor

briangregoryholmes commented Dec 21, 2023

Regarding your issue with the DropdownMenuSub, I agree that it's unintuitive. You have to also set disabled={true} on the SubTrigger and then you can add a control function via on:click

@ericpgreen2 Here's what I had to do to get it work:

 <DropdownMenuSub bind:open={subOpen} >
      <DropdownMenuSubTrigger
        disabled={true}
        on:click={() => (subOpen = !subOpen)}>View as</DropdownMenuSubTrigger
      >
      <DropdownMenuSubContent
        class="min-w-[150px] max-w-[300px] min-h-[150px] max-h-[190px] flex flex-col"
      >
        <ViewAsUserPopover
          organization={$page.params.organization}
          project={$page.params.project}
        />
      </DropdownMenuSubContent>
    </DropdownMenuSub>

@ericpgreen2
Copy link
Contributor Author

Regarding your issue with the DropdownMenuSub, I agree that it's unintuitive. You have to also set disabled={true} on the SubTrigger and then you can add a control function via on:click

Oh, nice find. Works 👍

@ericpgreen2 ericpgreen2 merged commit 7ccbefa into main Dec 21, 2023
4 checks passed
@ericpgreen2 ericpgreen2 deleted the use-shadcn-dropdown-in-cloud-ui branch December 21, 2023 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Style the Scheduled Report dropdown menus according to new mocks
3 participants