-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Allow debouncing/throttling x-model when using x-modelable #4186
base: main
Are you sure you want to change the base?
Conversation
For some clarity, the outer value means the theoretical state side (like a normal components data), while the inner value means the x-modelable (like the inputs value)? |
Yes, sorry. outer and innner are the terms used in the codebase. If we look at this example: <div x-data="{ number: 5 }">
<div x-data="{ count: 0 }" x-modelable="count" x-model.debounce="number">
<button @click="count++">Increment</button>
</div>
Number: <span x-text="number"></span>
</div> then outer is This behaviour would be consistent with how a normal |
The other option I have just throught of is to emit "input" events from the modelable. I have no Idea if this would work, but it would allow the x-model code to remain unchanged. I will try this later this evening as it is a more elegant solution :) |
Looking at the code, it isn't possible to use I could register some custom event listener on the |
I do actually think switching to an event is the smart way. Especially for it to support the |
Is there pesident for what I should name the event because it cant be edit: |
Latest commit uses an event based system. I am happy to switch this PR to either. Both have their flaws and neither is particularly eligant. setWithModifiers:With the new setter you are repeating the debounce/throttle handler code in another place (although I think this could be fixed by abstracting this code into a seperate function so it is clear what it's purpose is. It also adds a new item to the Event based:For the event based system you are forced to register a new event. It is mildly less performent as an event is being dispatched for every update. And the benifits interms of the event being able to bubble is lost by the fact that Personally, I prefer using the setter (and it's what I will continue to use in my projects for the time being). If this is the prefered option then I would also like to abstract out the throttle/debounce handler code (or maybe the whole handler code at the start of |
For your project, is there a reason why you can't debounce the internal x-model (just to understand the use case)? |
(wrong account sorry - reposting)
I have a min-max slider which are already being modelled with two internal values. To prevent the sliders from moving too close to one another I also have some code binding to the input event. There are 2 different locations which the user can enter values. The slider and a set of number input boxes (which is why I then want to debounce this input back out to a set of search filters (and also be able to send values back in - hence using model). I originally used events but it was clunky and hard to read as well as having to bind events to the window due to the layout of the input. I therefore also had to add special logic to make sure the events where coming from the right min-max slider. A mess! Now the code looks something like the following: <div class="w-96 max-w-full"
x-data="minMaxSlider(<?php echo $minPrice; ?>, <?php echo $maxPrice; ?>, <?php echo $gap; ?>)"
x-init="setMinValue(minPrice); setMaxValue(maxPrice)" x-modelable="[minValue, maxValue]" x-model.debounce="[minPrice, maxPrice]">
<div class="flex justify-between mb-4">
<div>
<label for="min-price-text-input">min</label>
<input id="min-price-text-input" type="number" min="<?php echo $minPrice; ?>" max="<?php echo $maxPrice; ?>"
value="<?php echo $minPrice; ?>" step="<?php echo $step; ?>" @input="
setMinValue($el.value);
$el.value = minValue;
" x-model="minValue">
</div>
<div>
<label for="max-price-text-input">max</label>
<input id="max-price-text-input" type="number" min="<?php echo $minPrice; ?>" max="<?php echo $maxPrice; ?>"
value="<?php echo $maxPrice; ?>" step="<?php echo $step; ?>" @input="
setMaxValue($el.value);
$el.value = maxValue;
" x-model="maxValue">
</div>
</div>
<div class="relative">
<div class="h-1 w-full bg-grey relative">
<div class="h-full inset-0 absolute rounded bg-black"
:style="`left:${((minValue - min) / (max-min)) * 100}%; right:${100 - ((maxValue - min) / (max-min)) * 100 }%;`">
</div>
</div>
<div class="absolute w-full h-1 top-1/2 left-0 -translate-y-1/2 pointer-events-none">
<label for="min-price-slider-input" class="hidden">minimum price</label>
<input id="min-price-slider-input" class="absolute w-full inset-0 h-1 pointer-events-none appearance-none"
style="background:none" type="range" min="<?php echo $minPrice; ?>"
max="<?php echo $maxPrice; ?>" value="<?php echo $minPrice; ?>" step="<?php echo $step; ?>" @input="
setMinValue($el.value);
$el.value = minValue;
" x-model="minValue">
</div>
<div class="absolute w-full h-1 top-1/2 right-0 -translate-y-1/2 pointer-events-none">
<label for="max-price-slider-input" class="hidden">maximum price</label>
<input id="max-price-slider-input" class="absolute w-full h-1 inset-0 pointer-events-none appearance-none"
style="background:none" type="range" min="<?php echo $minPrice; ?>"
max="<?php echo $maxPrice; ?>" value="<?php echo $maxPrice; ?>" step="<?php echo $step; ?>"
@input="
setMaxValue($el.value);
$el.value = maxValue;
" x-model="maxValue">
</div>
</div>
</div> with this JS class MinMaxSlider {
constructor(min, max, gap) {
this.min = min;
this.max = max;
this.gap = gap;
this._maxValue = max;
this._minValue = min;
}
get maxValue() {
return this._maxValue;
}
setMaxValue(value) {
value = parseFloat(value);
if (value - this._minValue < this.gap) {
this._maxValue = this._minValue + this.gap;
} else if (value > this.max) {
this._maxValue = this.max;
} else {
this._maxValue = value;
}
}
get minValue() {
return this._minValue;
}
setMinValue(value) {
value = parseFloat(value);
if (this._maxValue - value < this.gap) {
this._minValue = this._maxValue - this.gap;
} else if (value < this.min) {
this._minValue = this.min;
} else {
this._minValue = value;
}
}
} (edited for brevity) I also think that (even if my use case is insuficient), either a docs change or a code change is required. The docs specifically state that
and currently this is not how it behaves |
The point is that this PR, not being very robust (as you said, both approaches as you said are currently sub-optimal), needs a lof of demand to be merged. Especially because there's a way to do it in userland. Not relevant to the whole discussion but instead of adding an @input on each input field (which competes/conflicts with x-model) you should look at using x-effect on a single element to call your functions. x-model is already doing 2 ways binding so stuff like |
This reverts commit c00bee0.
not relevant to this PR but I found that without As for using debounce on the inner To the PR itself. The more I think about it the more I think the original way is correct and I apologise for the confusion. The actual change is very minor and it has been wrapped up in more words than are neccassary because I have been overthinking the whole thing. I have reverted the change to how I think it should be. The only modifications have been:
Overall pros
Overall cons
I apologise for the confusion. I hope you can see the intended original purpose of the PR and the minimal effect this has other than fixing a bug and adding some tests. The code has about 4 lines that have been changed. It is robust (I have been using it this past week), I just didn't have the confidence to stick to my original solution. |
Yeah, it would be a more major rewrite of the whole feature... |
I believe I have the same issue here... @SimoTod you mentioned we should just debounce the internal input, but I dont have that option here. This is my use case, for a tiptap integration. Ultimately i am trying to debounce the wire:model on the parent div. The plain input debounces as expected. The x-modelable one does not. <div x-data="{value: $wire.form.body}"
x-init="$watch('value', value => console.log('value', value))"
wire:model.live.debounce="form.body"
>
<input class="debounced-input" x-model.debounce="value" type="text"/>
<div class="editor" wire:ignore x-data="tiptapEditor(value)" x-model.debounce="value" x-modelable="markdown">
<div x-ref="editorReference"></div>
</div>
</div> Entangle version: <div x-data="{value: $wire.entangle('form.body').live}">
<input class="debounced-input" x-model.debounce="value" type="text"/>
<div class="editor" wire:ignore x-data="tiptapEditor(value)" x-model="value" x-modelable="markdown">
<div x-ref="editorReference"></div>
</div>
</div> https://stackoverflow.com/questions/79121535/livewire-alpine-v3-debounce-on-x-modelable-not-working |
Solves
#2812
Problem
When using
x-model.debounce
with x-modelable the outer values will not be debounced. I belive this to be a bug as the documentation says the following about x-modelable:Solution
I have not modified the
el._x_model.set
function as you would expect this to ignore the throttle/debounce (the same way it does for normalx-models
). Instead I have added an extra method (setWithModifiers
) which respects debounce and throttle modifiers. I have then changed the outerSet of x-modelable to usesetWithModifiers
(still allowing the inner value to update live).Tests
I have included one test for debounce and throttle respectively (as they are implemented slightly seperate from each other, I thought this reasonable).