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

Add |local to Drawer/Dialog/Backdrop/Menu without breaking ToggleButton usage #51

Closed
techniq opened this issue Apr 4, 2023 · 1 comment

Comments

@techniq
Copy link
Owner

techniq commented Apr 4, 2023

Recently all Svelte transition usage within svelte-ux was updated to include the |local modifier to not break SvelteKit page transitions (see: sveltejs/svelte#6686, sveltejs/kit#628, sveltejs/kit#8670, and sveltejs/kit#5321.

In doing so, this breaks Dialog and Drawer transitions (particularly in) with ToggleButton since the {#if} within Dialog / Drawer are closer than the one within ToggleButton and thus are no longer treated as local.

One of the benefits of using ToggleButton to conditionally render a Dialog/Drawer is to handle mounting/unmounting even when the component is within a wrapping component (ex. UserDrawer, FilterDrawer, etc) as otherwise the wrapping component will always be mounted / reactive.

  • ToggleButton
    • UserDrawer
      • Drawer

Since Dialog/Drawers are modals, adding |local to fix SvelteKit page transitions shouldn't be needed as you need to dismiss the modal before clicking on a link, but there might be an overlooked use case. For now, the workaround will be to revert adding |local for these components until a better solution is found.

A few thoughts on possibly solutions:

  • Try to not add the {#if open} within Dialog/Drawer when used with ToggleButton
    • We still want it for all other use cases (as removing it completely will complicate them)
    • Trying to add it conditionally just adds another conditional
    • Exposing a separate component (DrawerBase or InternalDrawer or something) with most of the impl could work, but would be confusing to the user
  • Allow the user to opt-out of the {#if on} within ToggleButton, but along with being difficult without another conditional or nested slot, it would break the UserDrawer use case
@techniq techniq changed the title Add |local to Drawer/Dialog without breaking ToggleButton usage Add |local to Drawer/Dialog/Backdrop without breaking ToggleButton usage Apr 4, 2023
techniq added a commit that referenced this issue Apr 4, 2023
@techniq techniq changed the title Add |local to Drawer/Dialog/Backdrop without breaking ToggleButton usage Add |local to Drawer/Dialog/Backdrop/Menu without breaking ToggleButton usage Apr 4, 2023
@techniq
Copy link
Owner Author

techniq commented Apr 4, 2023

After some thought, we could split transition: into in: and out: and only apply |local to the out: (which is what breaks SvelteKit navigation, but the in: appears to be only what's broken with ToggleButton (needs more investigation though).

@techniq techniq closed this as completed in 14a3c57 Apr 6, 2023
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

No branches or pull requests

1 participant