Skip to content

Conversation

@Anoesj
Copy link

@Anoesj Anoesj commented Mar 28, 2025

I worked on a big rewrite of vaul-vue this week, because I ran into limitations, all kinds of bugs and lack of customization.

This rewrite has a lot of changes, but I think they're all for the better:

  • defaultOpen prop is deprecated in favor of open attribute, :open="true".
  • Because of using defineModel instead of useVModel, the open prop now has less control limitations. If someone uses the :open="ref" way, the dialog still closes on escape/click outside/DrawerClose click, but the ref does not get updated. However, if the prop is updated by the parent component, the drawer does react to that.
  • Using defineModel for activeSnapPoint as well.
  • CSS based wrapper styling. Instead of setting CSS properties with JS, we only set a CSS variable called --vaul-closed-percentage on the wrapper, which is used by the default styling to apply translate/scale etcetera that works, even when you resize your screen while the drawer is open.
  • Users can set --vaul-duration and --vaul-easing on their :root to adjust transition durations and easings.
  • Users can now use --vaul-closed-percentage to apply custom styling on their drawer wrappers.
  • Users can now override --vaul-wrapper-border-radius to apply a custom target border-radius on their drawer wrappers. This does not have to be in px, but can be %/rem/em etcetera. E.g. when setting --vaul-wrapper-border-radius: 2rem the drawer will have a border-radius of 2rem when the drawer is open.
  • Users can now override --vaul-wrapper-scale-px-unitless to apply a custom target shrink/grow on their drawer wrappers. Values have to be unitless, such as -30 (shrinks wrapper 30px when drawer is open), 40 (grows wrapper 40px when drawer is open).
  • Nested drawers have correct styling in all directions now.
  • Nested drawers always have shouldScaleBackground set to false now, to prevent issues with a parent drawer that also scales the background.
  • Fixed some bugs regarding async drawer components that have open/:open="true"/:v-model:open="true" on initial setup.
  • Prefix any existing CSS variables that vaul-vue sets with --vaul, rename animations to kebab case with the vaul prefix as well.
  • Warn in the console if the drawer wrapper couldn't be found.
  • Got rid of unused variables, types, interfaces etc.
  • Got rid of package-lock.json, since we use pnpm.
  • Tests all still pass.
  • Playground has been extended a little, especially for nested drawers.
  • All dependencies and corresponding configs updated.

TODO:

  • On mobile devices, :active on the drawer itself does not seem to work, so transitions do not get disabled on the drawer wrapper when the drawer is being dragged.
  • Do not use setTimeout, use transitionend/animationend events instead if possible.
  • Try to use CSS variables for drawer and nested drawer translations as well.

A little demo of what's possible with this approach:
Screencast from 30-03-25 10:46:51.webm

Anoesj added 7 commits March 22, 2025 13:55
…ration and easing and border radius configurable using CSS vars, deprecate `defaultOpen` (use `open` or `:open="true"`, update deps
… `v-if` and initially open, make sure latest `drawerEl` element is re-retrieved in `setTimeout`, fix broken `onNestedOpenChange` call
@changeset-bot
Copy link

changeset-bot bot commented Mar 28, 2025

⚠️ No Changeset found

Latest commit: d33782c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Anoesj
Copy link
Author

Anoesj commented May 6, 2025

Any feedback would be greatly appreciated!

@sadeghbarati sadeghbarati requested a review from zernonia May 6, 2025 10:53
defaultValue: props.defaultOpen,
passive: (props.open === undefined) as false,
}) as WritableComputedRef<boolean>
const open = defineModel<boolean>('open', {
Copy link
Collaborator

Choose a reason for hiding this comment

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

defineModel is Vue 3.4+ feature, I also would prefer to use defineModel and no longer worry about compatibility with older versions of Vue

However Reka-ui is still using useVModel in the codebase so let's use useVModel instead

Copy link
Author

Choose a reason for hiding this comment

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

Ah that's a shame, but a valid point. I see Reka UI requires Vue ^3.2.0. Interestingly, vaul-vue seems to target Vue ^3.3.0 in its peer dependencies, but also lists Vue ^3.4.5 in the regular dependencies? So besides listing it twice, it doesn't seem to follow the same requirements as Reka UI. What is your vision on that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That is a mistake, that vue is in the dependencies in vaul-vue package.json it should be only in peerDependencies

See https://github.com/emilkowalski/vaul/blob/main/package.json#L62

Copy link
Author

Choose a reason for hiding this comment

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

Ah it's just copied over from the React version. Are you planning on fixing this one yourself, or do you want a separate PR or have it included in this one?

Copy link
Collaborator

@sadeghbarati sadeghbarati May 6, 2025

Choose a reason for hiding this comment

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

Can I? I would glad to help on this case,
I'll push a commit in this PR if you allow :)

Copy link
Author

Choose a reason for hiding this comment

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

Of course! Any help finishing up this PR is very welcome 😄

@sadeghbarati
Copy link
Collaborator

Hi, Thanks for the PR and detailed explanation

@sadeghbarati
Copy link
Collaborator

There are still some changes that need to consider for example NestedDrawer example not working properly sometime (black background with and without my new changes)

Also NestedDrawer example, Transition in test/nested-drawer is a bit different from the default NestedDrawer example

I didn't check the rest of the examples will check them later

@sadeghbarati
Copy link
Collaborator

initial-snap and with-snap-points examples also has a problem, when you close the drawer, the drawer will never be open after clicking on the button again

This issue happens with and without my changes

@sadeghbarati sadeghbarati removed the request for review from zernonia May 7, 2025 07:08
@sadeghbarati
Copy link
Collaborator

It would be nice if you re-check the upstream vaul and see if you can fix these issue (whenever you have free time ofcourse)

@metkm
Copy link

metkm commented Aug 13, 2025

well i like the CSS-based approach, but the problem is that it has performance issues.
image

Recalculate style is taking so long because of the usage of setProperty. I guess when you use custom css property it triggers layout calculations. I'm seeing this performance issue on a physical android device.

Also found this related issue
https://stackoverflow.com/questions/73568220/javascript-updating-css-custom-properties-slower-than-using-element-styles

This issue only gets worse if you have more children in your main component.

@metkm
Copy link

metkm commented Aug 14, 2025

I have now forked this PR & Library and am doing a refactor with performance in mind. Want to make it so they have the same features. Currently, opening and closing the sheet costs no performance hit. (tested on physical device, debug mode)

image

@Anoesj
Copy link
Author

Anoesj commented Aug 15, 2025

I have now forked this PR & Library and am doing a refactor with performance in mind. Want to make it so they have the same features. Currently, opening and closing the sheet costs no performance hit. (tested on physical device, debug mode)

image

Cool! Keep us posted 😄. Could you post a link to your fork?

@metkm
Copy link

metkm commented Aug 15, 2025

@Anoesj

https://github.com/metkm/vaul-vue/tree/css-based
I haven't deleted the unused old code yet because, currently, it helps me to look at how stuff is implemented. All the changes i made are in the src/composables folder. I will also use CSS variables later for animation customization, but didn't implement them yet.

I don't think I can pass the offset value as a CSS variable because of the performance cost. I think I will pass that value to slot. Similar to how vueuse does.

https://vueuse.org/core/useWindowSize/#component-usage

current performance of the drawer, playing like crazy with it. Average of 5ms. Probably can be optimized more
image

@metkm
Copy link

metkm commented Aug 19, 2025

I wonder if desktop users would want this experience. Dragging with the mouse and collapsing the drawer (not using scroll wheel). I've implemented this custom scroll because I wanted to be able to close the drawer even when scrolling is started and reaches the end.

2025-08-19.23-19-42.mp4

Another solution to this is using touch events on mobile and pointer events on desktop. But it looks like safari doesn't support TouchEvents for some reason.

@metkm
Copy link

metkm commented Aug 27, 2025

Link to the pr #111

@Anoesj
Copy link
Author

Anoesj commented Aug 27, 2025

Link to the pr #111

Thanks for sharing! It would help maintainers to understand your PR if you could write down all changes and whether they are breaking or not. I haven't got the time right now to check your PR, but I'm curious to see the results! 😄

@metkm
Copy link

metkm commented Aug 27, 2025

Link to the pr #111

Thanks for sharing! It would help maintainers to understand your PR if you could write down all changes and whether they are breaking or not. I haven't got the time right now to check your PR, but I'm curious to see the results! 😄

Yeah, I'm going to add a description sometime. Props and stuff 90% the same. Using it is not much different

@Anoesj
Copy link
Author

Anoesj commented Aug 28, 2025

Yeah, I'm going to add a description sometime. Props and stuff 90% the same. Using it is not much different

Great! Is usage substantially different from the current version of vaul-vue and/or usage with the breaking changes in my PR?

And another question: do you think we could finalize my PR (together) and then continue on yours, or would that be suboptimal?

@metkm
Copy link

metkm commented Aug 28, 2025

Great! Is usage substantially different from the current version of vaul-vue and/or usage with the breaking changes in my PR?

Nope, Usage should be pretty much the same. Except I don't have some new CSS variables that you have. Should be pretty easy to add them, though.

And another question: do you think we could finalize my PR (together) and then continue on yours, or would that be suboptimal?

I think it would be suboptimal because I moved stuff around. We can merge this and then continue on my PR just to get them deleted because I've moved them on my PR lol. I think the best way is just to add CSS variables to my PR and continue from there.

When you have time, you can compare them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants