extensions icon indicating copy to clipboard operation
extensions copied to clipboard

MtxSlider no way to control click event

Open andrecadete opened this issue 2 years ago • 2 comments

Hi,

I understand clicking on the slider bar is a feature, not a bug. However, since there's no way to control the click event on this element, we're left with what behaves like a bug.

If you click right on top of the Thumb circle, the values don't change but eventChanged() is triggered and changed emits. So far this is fine, you can just keep state of previous values yourself and wrap the change emission in your own method to prevent emitting if values didn't actually change, but it causes the following issue:

If you click the thumb and immediately drag it triggers both eventChanged() and the changed event emitter as stated previously, but because you dragged it, it will contain the new values so we have no way to prevent this submission. Mind you, the thumb circle is still being dragged when this change is emitted.

A way to override the click handler of either the slider bar or the thumb buttons themselves could solve this, so we can for instance event.stopProgagation() if the clicked element was the thumb circle and not the slider bar itself. I understand we need this click handling on the bar so that the thumbs slide to the clicked place, based on which thumb is closer, but it shoudn't trigger when clicking the thumb, only when letting go (which already happens).

If there are other solutions, happy to hear about them.

Example:

// component.ts
@Output() change = new EventEmitter<number | number[]>();
private previousValue: number | number[] | null = null;

// change handler
emitChange(event: MtxSliderChange) {
    console.log('Change emitted by MtxSlider');
    const previousValue = this.previousValue;
    const value = event.value;
    let valueChanged: boolean;
    
    if (isArray(previousValue) && isArray(value)) {
      valueChanged = previousValue[0] != value[0] || previousValue[1] != value[1];
    } else {
      valueChanged = previousValue != value;
    }

    if (value && valueChanged) {
      console.log('EMITTING CHANGE!');
      this.change.emit(value);
    }
  }
// template
<mtx-slider
  [name]="name"
  [disabled]="disabled"
  [max]="max"
  [min]="min"
  [step]="step"
  [thumbLabel]="thumbLabel"
  [displayWith]="formatLabel"
  [(ngModel)]="value"
  [attr.aria-labelledby]="labelId"
  (change)="emitChange($event)"
  [value]="value"
>
</mtx-slider>

Steps to reproduce:

  • Click precisely on the thumb circle and let go
// Change emitted by MtxSlider
  • Click precisely on the thumb circle and immediately drag it - console.log will fire
// Change emitted by MtxSlider
// EMITTING CHANGE!

This should only happen on dragend or mouseup, but it happens on click. 99% sure this is just the click handler of the slider bar that doesn't stopPropagation when it should.

Edit: it's easily triggered if you increase the size of the thumb circle. For my current project, I wanted them slightly bigger. This makes it so the click handler detects a click "too far" from the current position (I'm assuming) and triggers the thumb to move and thus the change emitter. Nonetheless, this prevents custom styling whereas a simple event.stopPropagation() / event.stopImmediatePropagation() in a click handler specifically for the thumb elements would solve the issue (tbh this could already be part of the default functionality).

andrecadete avatar Feb 08 '22 12:02 andrecadete

I found a hacky workaround, in case someone else has the same issue:

  • Trigger the handler in both (mouseup) and (change), but pass a boolean flag to the method (you won't need the event param, we're accessing the slider via ViewChild() to re-use the same method for both cases);
// template
<mtx-slider
  [name]="name"
  [disabled]="disabled"
  [max]="max"
  [min]="min"
  [step]="step"
  [thumbLabel]="thumbLabel"
  [displayWith]="formatLabel"
  [(ngModel)]="value"
  [attr.aria-labelledby]="labelId"
  (change)="emitChange()"
  (mouseup)="emitChange(true)"
  [value]="value"
  #mtxSlider
>
</mtx-slider>
  • Add some conditions to your handler:
    • If coming from (change), ensure the prop "_isSliding" is FALSE before emitting.
    • if coming from (mouseup), emit regardless of "_isSliding".
// component.ts
@Output() change = new EventEmitter<number | number[]>();

private previousValue: number | number[] | null = null;
@ViewChild('mtxSlider') mtxSlider?: MtxSlider;
  
emitChange(emitWhileSliding = false) {
    const previousValue = this.previousValue;
    const value = this.mtxSlider?.value;
    const isSliding = this.mtxSlider?._isSliding ?? false;

    let valueChanged: boolean;

    if (isArray(previousValue) && isArray(value)) {
      valueChanged = previousValue[0] != value[0] || previousValue[1] != value[1];
    } else {
      valueChanged = previousValue != value;
    }

    // if from mouseUp event, it will come in as "sliding=true" due to a bug.
    if (value && valueChanged && (!isSliding || emitWhileSliding)) {
      console.log('EMITTING CHANGE!');
      this.change.emit(value);
    }
  }

Long story short, if your thumb is wider than the default and you click on it off-centered, the change emitter from MtxSlider will emit. However, the "_isSliding" property will correctly be true, so we can prevent emitting in our wrapper if that's the case (it means the user is still dragging, didn't drop yet). When using the (mouseup) handler, it will obviously not trigger when clicking the element regardless of size or position until you let go of it, which is the functionality that should prevail when clicking on the thumb itself and not the bar regardless of the thumb's size. Interestingly the moment the (mouseup) event is triggered, the prop _isSliding is still incorrectly set to true, fortunately this is not an issue, since we know this is only fired when letting go of the thumb, so we can emit as long as there are changes.

andrecadete avatar Feb 08 '22 13:02 andrecadete

Many thanks for your advice, I'll have a try.

nzbin avatar Mar 29 '22 01:03 nzbin

mtx-slider has been removed since 16.2.0, please use mat-slider instead.

nzbin avatar Dec 13 '23 04:12 nzbin