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

Move dialogs to Radix #3470

Open
mary-ext opened this issue Sep 19, 2024 · 3 comments
Open

Move dialogs to Radix #3470

mary-ext opened this issue Sep 19, 2024 · 3 comments
Assignees
Labels
Module: Design System Type: Chore Tedious work that needs to be done but doesn't have intrinsic value

Comments

@mary-ext
Copy link
Member

No description provided.

@mary-ext mary-ext added Type: Chore Tedious work that needs to be done but doesn't have intrinsic value Module: Design System labels Sep 19, 2024
@mary-ext mary-ext self-assigned this Sep 19, 2024
@mary-ext
Copy link
Member Author

mary-ext commented Sep 23, 2024

From what I can tell, it seems impossible for us to keep the same structure as before where dialogs can be used like this:

<PreviewDialog v-if="open" />

We'd have to move to controlling the open prop to have dialogs transition as expected, but the problem with that is <PreviewDialog /> here gets permanently rendered now. It's really iffy for component logic to be dealing with whether the dialog is open or not, so this is a no-go.

There's three alternatives here:

1. Move all component logic into an "inner" component, e.g. <PreviewDialogInner />

If this was React, I'd honestly go for this approach, but we're using Vue with the SFC scheme, so each dialogs will have two files each and that doesn't seem good.

2. Move the dialog component outside

This would look something like this

<Dialog :open="open">
  <PreviewDialog />
</Dialog>

Honestly at that point we might as well just go fully into Radix' approach here and have the states controlled by Radix.

<Dialog>
  <template #trigger>
    <button>click here<button>
  </template>

  <PreviewDialog />
</Dialog>

3. Roll our own component using built-in <dialog> element

That's an option now, I believe we can do v-if on a component that contains <Transition /> inside. caniuse reports 95% so we shouldn't run into much issues but I'd rather not though if our priority is accessibility.

@mary-ext
Copy link
Member Author

mary-ext commented Oct 22, 2024

Looking into this again, it might be unwise to roll our own modals here but I'm not sure there's any other choice.

Radix (React) has their components split into individual packages including their internal components, we don't have this for Radix Vue, so we'd need to patch the package.json file to get access to FocusScope, FocusGuard, and DismissableOverlay, those are the 3 components we need for a dialog implementation.

We should keep the dialog component generic enough so that we can reuse it for the search dialog, which right now is the only thing that isn't making use of our design system's Dialog component.

Bluesky went for this approach for their web-based dialogs, so we know that it works well. We'd just have to watch out for breaking changes that might happen as a result, as they are, internal components.

@mary-ext
Copy link
Member Author

Bad news: Radix Vue is bundled, I'm not sure if there's any good way of doing this then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Design System Type: Chore Tedious work that needs to be done but doesn't have intrinsic value
Projects
Status: In progress
Development

No branches or pull requests

1 participant