ng-matero / extensions

Angular Material Extensions Library.
https://ng-matero.github.io/extensions/
MIT License
393 stars 48 forks source link

MtxSlider no way to control click event #86

Closed andrecadete closed 5 months ago

andrecadete commented 2 years ago

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:

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 commented 2 years ago

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

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.
nzbin commented 2 years ago

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

nzbin commented 5 months ago

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