-
Notifications
You must be signed in to change notification settings - Fork 31
Refactor #111
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
Open
metkm
wants to merge
108
commits into
unovue:main
Choose a base branch
from
metkm:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Refactor #111
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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
Author
3 tasks
Author
|
Note: Or not that but a better way of setting styles. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Wanted to work on this PR because of performance reasons. Tested it on Nuxt UI's page with a strong physical device, and the performance was bad. So I forked the PR that looked interesting by anoesj. That also came with other problems; CSS variables were causing performance issues, even though I liked the approach. So this approach is back to manually adding
translatestyle. This means you probably can't do weird effects.This PR also clears everything up. I've found that the old code was hard to read because everything was cramped into
controls.ts. I've split some stuff up with different composables.Also switched a lot of variables to use
computedbecause they are cached, which means we don't lose performance, and pass values to slots, which users can do whatever they want with them. This also means everything should be updated accordingly with any changes. E.g.: changing and resizing the content size of the drawer, window, orrefvariables, etc.There are also some features that didn't exist before, like the scroll implementation I've done so that the drawer drags when scrolling content at the top. Just like any native drawer would do on mobile. This can be done using
data-vaul-scrollableand nothing else is needed.Removed the
NestedDrawercomponent. Now you can use the same components with just nesting them, seetests/NestedDrawer.vue. You don't need to do anything else. This is because I use Stacks to keep track of open drawers. seecomposables/useStacks. I'm not sure if you could do this before, but now you can open 30 different drawers with all having different props, etc. There should not be any problems.There might be some missing props or bugs. This PR is ready to review, but I wouldn't merge it yet before testing it.