search

radix-vue / vaul-vue

An unstyled drawer component for Vue.
https://vaul.unovue.com
188 stars 8 forks source link

Reduce duplicated utilities #12

Closed sadeghbarati closed 6 months ago

sadeghbarati commented 8 months ago

Thanks for take care of porting vaul to Vue

radix-vue uses @vueuse/core underhood so why not use their utility functions?

why not using our local utility functions?

They have same logic why not replace them with already defined utility in @vueuse/core? we can prevent duplication by doing this

Local utils @vueuse/core utils
isIOS function isIOS expression
addEvent useEventListener
set function CSS maybe useCssVar I'm not sure about this one

Or other local utility which I can't recognize the equivalent in @vueuse/core

Elliot-Alexander commented 8 months ago

Yeah I think this a great idea, cleans up quite a few utility functions :D

Elliot-Alexander commented 8 months ago

Going through this, I have a suspicion that we can remove a lot of the util if we use the useScrollLock as it appears to resolve most of what those utils work to achieve. @connerblanton what are your thoughts? I raised #21 which includes this change and tested on old iPhone 8 I have (couldn't see any issues but that phone is relatively old)

connerblanton commented 8 months ago

I think we can definitely do this. However, I do see a problem (I don't think it's caused by this, rather we just have some more work to do for it) when there are inputs and scrollable content. I took the code from the Scrollable with inputs example from Vaul. When I test this example on ours, we automatically focus on the first input and when the keyboard is up, I am able to scroll and the page refreshes (on Safari) when I pull down.

Again, I think we can still use this, but we also need to make this example work

Elliot-Alexander commented 8 months ago

Ah I see, so this is limited to when the drawer is scrollable and contains input field(s)? You're right then, we'll definitely have to do some work on this