Open Spitfire972 opened 4 months ago
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)?
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 number
and inner is count
. In this example if number
was set directly then count
would update immedietly, and if count
was set then number
would update whilst respecting debounce.
This behaviour would be consistent with how a normal x-model
works with input elements.
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 input
events to send the update and then using the on()
helper to detect it and manage the modifiers. This is because, as part of setting up the x-modelable
, the input
/change
event listener is removed. And if I were to add it back then it would cause the the x-model
to capture input events (which is unwanted behaviour).
I could register some custom event listener on the x-model
which x-modelable
could interact with. However, I don't think this is a good idea as I couldn't find any other examples of a custom event in the code, and all it would do is save repeating the debounce registering code in two places (on()
and now x-model
)
I do actually think switching to an event is the smart way.
Especially for it to support the x-model
not being on the same element as the x-modelable
. which can work nicely with components
I do actually think switching to an event is the smart way.
Especially for it to support the
x-model
not being on the same element as thex-modelable
. which can work nicely with components
Is there pesident for what I should name the event because it cant be input
as that would cause the x-model
to act normally. I do think that the ability for it to bubble up is useful though. For now I will name it something like "x-modelable-input"
and quickly implement it, but welcome to suggestions
edit:
The "bubbling" up feature won't work as x-modelable
specifically checks if x-model
is on the element before initialising
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.
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 _x_model
object on the element which may not be ideal.
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 x-modelable
explicitally checks whether there is also an x-model
attribute on the element, (and implementing it to allow bubbling would be impossible for setters as you do not know where the child with x-modelable
is in the DOM e.g. there could be multiple etc.) (also if this was the behaviour you wanted then it would be better to use $dispatch
and x-on
or just move the x-model
lower in the tree)
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 on()
, which also supports once
, prevent
, and stop
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)
For your project, is there a reason why you can't debounce the internal x-model (just to understand the use case)?
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 x-model
internally is alot easier than repeated event bindings and dispatches for each input).
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
It's [x-modelable] useful for abstracting away Alpine components into backend templates and exposing state to the outside through x-model as if it were a native input.
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 exactly the same but what you described can be achieved by putting the debounce modifier on the internal input i.e. x-model.debounce="minValue"
.
I appreciate they are not debounced between each other but a user won't be that fast anyway.
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 $el.value = maxValue
doesn't make much sense.
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 exactly the same but what you described can be achieved by putting the debounce modifier on the internal input i.e.
x-model.debounce="minValue"
. I appreciate they are not debounced between each other but a user won't be that fast anyway.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
$el.value = maxValue
doesn't make much sense.
not relevant to this PR but I found that without $el.value = maxValue
it failed to update to the maximised value for that given element. I think this is because my code gets run, then the input event is run on x-model
which reset the value prior to when my code ran. I don't see an issue with an input event on the element as they will each handle the event seperately and dont opperate on the same values (there is no setter on the maxValue).
As for using debounce on the inner x-model
- this is not a very elegant result for the user, as when they move a slider they get no visual feedback from the number input, e.g. how will they know what value they are currently at, isn't it jaring when the slider stops and suddenly the number changes. I didn't take this solution as I knew the client of the website would have asked me to make it smooth.
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:
setWithModifiers
to x-model
setWithModifiers
in x-modelable
x-modelable
behaves how it is expected to from the documentation_x_model
has an extra member which I guess could be a pro or a con (personally I would see this as a pro but leaving it here)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.
The "bubbling" up feature won't work as x-modelable specifically checks if x-model is on the element before initialising
Yeah, it would be a more major rewrite of the whole feature...
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).