Skip to content
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

MatSlider: change event fired twice #14363

Closed
PhilippS93 opened this issue Dec 3, 2018 · 19 comments · Fixed by #20240
Closed

MatSlider: change event fired twice #14363

PhilippS93 opened this issue Dec 3, 2018 · 19 comments · Fixed by #20240
Assignees
Labels
area: material/slider P2 The issue is important to a large percentage of users, with a workaround

Comments

@PhilippS93
Copy link

Bug:

MatSlider change event is fired twice when slider knob is dragged and released. Surprisingly enough, this happens only sometimes.

What is the expected behavior?

Event emitted when the slider value has changed.

What is the current behavior?

MatSlider change event is fired twice when slider knob is dragged and released.

What are the steps to reproduce?

See: stackblitz

What is the use-case or motivation for changing an existing behavior?

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Is there anything else we should know?

@manklu
Copy link

manklu commented Dec 3, 2018

It fires immediately if you don't click exactly in the center of the knob.

@crisbeto
Copy link
Member

crisbeto commented Dec 6, 2018

It probably because we change the value on mousedown and then while the user is dragging. This sounds like it's working as expected.

@cmedinadeveloper
Copy link

This is happening to me on every single iOS devices, any suggestions?

@notbenj
Copy link

notbenj commented Feb 15, 2019

It probably because we change the value on mousedown and then while the user is dragging. This sounds like it's working as expected.

I'm sure an end-user doesn't expect the value to be changed, just because he/she grabbed the knob 1 pixel on the left of the knob center :)

I think the behavior should be on the mouseup event, to avoid this situation.

@crisbeto
Copy link
Member

The problem, if we were to do that, is that the events will no longer be in sync with what is being rendered on screen.

@Domvel
Copy link

Domvel commented Mar 20, 2019

Confirmed, it's an unexpected behavior (bug).
https://stackblitz.com/edit/angular-mat-slider-full-options?embed=1&file=app/slider-configurable-example.html

If you click on the thumb if the slider is not focused an begin to drag the change event is emitted.
Also if you slick on the range (line, not the thumb) the change event is always fired on mouse button down. And final on pointer (mouse / touch) up (as expected).

Whatever the component does for internal reasons, the output interface "(change)" should only be fired once on pointer up. You can keep change detection on pointer down internal for the component. But not for the output change.

@crisbeto It's only about the output event of the component. You can do anything internal. Inside the component you can use mouse down whatever to render the current value. BUT the interface output (change) event should only be fired once. You could split the output event value with a internal value and only emit the value for output on mouse up / pointer up / etc.

@Domvel
Copy link

Domvel commented Mar 20, 2019

Here's the current behavior.
export
You see that the change event is fired when the user just press the mouse button down to begin dragging. This is an unexpected behavior. (bug) It's expected on mouse button up.
The yellow circle indicates the pressed mouse button.

@Domvel
Copy link

Domvel commented Mar 20, 2019

Workaround:
Use the event slideend.

<mat-slider
  #mySlider
  (slideend)="sliderOnChange(mySlider.value)"
></mat-slider>

This is the "real" (expected) "change"-event.
I use the same event like the slider does.
https://github.com/angular/material2/blob/7.3.5/src/lib/slider/slider.ts#L121

UPDATE:
Known issue: With (slideend) a simple click on the range-line of the slider does not fire the slideend until it has never been dragged. #2767
Alternative you can listen on the (mouseup) event or (pointerup). With these events you have to implement a change-detection.
You'll also have problem if your cursor is not overlapping the component on pointerup. Therefore you have to handle cancle events. And and and... Just fix this issue please 😄

UPDATE:
I found a solution for the slideend / pointerup issue. Just combine it with a simple change-detection.

<mat-slider
  #mySlider
  (slideend)="sliderOnChange(mySlider.value)"
  (pointerup)="sliderOnChange(mySlider.value)"
></mat-slider>
sliderOnChange(value: number) {
  if (this.mySliderValue !== value) {
    this.mySliderValue = value;
    console.log('changed: ', value);
  }
}

Ok, now it works. THIS is the "real" "change"-event as expected. But just a workaround.

@BorisZubchenko
Copy link

Well, the slideend event is not available now :(

@sksuri100
Copy link

Bug:

MatSlider change event is fired twice when slider knob is dragged and released. Surprisingly enough, this happens only sometimes.

What is the expected behavior?

Event emitted when the slider value has changed.

What is the current behavior?

MatSlider change event is fired twice when slider knob is dragged and released.

What are the steps to reproduce?

See: stackblitz

What is the use-case or motivation for changing an existing behavior?

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Is there anything else we should know?

i have faced same problem please let me know what is right solution of this bug????

@mmalerba mmalerba added the needs triage This issue needs to be triaged by the team label May 20, 2020
@crisbeto crisbeto added area: material/slider P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed needs triage This issue needs to be triaged by the team labels May 27, 2020
@Sn3b
Copy link

Sn3b commented Jun 16, 2020

Well, the slideend event is not available now :(

I have a little work around that seems to work so far... Please don't fire at me if it doesn't for you ;)

<mat-slider min="0"
            max="100"
            step="1"
            (input)="sliderSliding( $event )"
            (change)="sliderChanged( $event )"
            (mouseup)="sliderMouseButtonReleased( $event )"
            [(ngModel)]="value">
</mat-slider>
    public value: number = 0;
    private _slidingValue: number;

    public sliderSliding( args: MatSliderChange ): void {
        this._slidingValue = args.value;
    }

    public sliderMouseButtonReleased( args: MouseEvent ): void {
        if ( this.value === this._slidingValue ) {
            this.sliderChanged( null );
        }
    }

    public sliderChanged( args: any ): void {
        if ( !!args && !!args.source._lastPointerEvent ) {
            return;
        }

        // Your code to process the event here
    }

@KrzysztofMilewski
Copy link

For those who - like me - desperately need a workaround: I downloaded the material repository and moved all necessary files to my project. Then, in src/material/slider/slider.ts in method _pointerDown (line 620) I removed line 644, which is this: this._emitChangeEvent();.

This works for my case, but bear in mind, that I haven't run any automated tests againist this change. Hope this helps

@samworley
Copy link

Well, the slideend event is not available now :(

I have a little work around that seems to work so far... Please don't fire at me if it doesn't for you ;)

<mat-slider min="0"
            max="100"
            step="1"
            (input)="sliderSliding( $event )"
            (change)="sliderChanged( $event )"
            (mouseup)="sliderMouseButtonReleased( $event )"
            [(ngModel)]="value">
</mat-slider>
    public value: number = 0;
    private _slidingValue: number;

    public sliderSliding( args: MatSliderChange ): void {
        this._slidingValue = args.value;
    }

    public sliderMouseButtonReleased( args: MouseEvent ): void {
        if ( this.value === this._slidingValue ) {
            this.sliderChanged( null );
        }
    }

    public sliderChanged( args: any ): void {
        if ( !!args && !!args.source._lastPointerEvent ) {
            return;
        }

        // Your code to process the event here
    }

Thanks! This helped me out a lot, but I noticed that you will still get multiple events fired when using a touch device or if using a keyboard to move the slider.
I found a simpler way of working around the issue is instead of binding to the change event, to bind to mouseup, keyup and touchend to cover keyboard, mouse and touch. If you do this you no longer need the sliderMouseButtonReleased function and can remove the return at the start of sliderChanged e.g.

<mat-slider min="0"
            max="100"
            step="1"
            (input)="sliderSliding( $event )"
            (mouseup)="sliderChanged( $event )"
            (keyup)="sliderChanged( $event )"
            (touchend)="sliderChanged( $event )"
            [(ngModel)]="value">
</mat-slider>
    public value: number = 0;
    private _slidingValue: number;

    public sliderSliding( args: MatSliderChange ): void {
        this._slidingValue = args.value;
    }

    public sliderChanged( args: any ): void {
        // Your code to process the event here
    }

@vanrado
Copy link

vanrado commented Aug 7, 2020

(mouseup)="sliderChanged( $event )"
            (keyup)="sliderChanged( $event )"
            (touchend)="sliderChanged( $event )"

touchend event is not emitted by mat-slider. Did you test that pls?

@samworley
Copy link

samworley commented Aug 7, 2020

(mouseup)="sliderChanged( $event )"
            (keyup)="sliderChanged( $event )"
            (touchend)="sliderChanged( $event )"

touchend event is not emitted by mat-slider. Did you test that pls?

Appeared to work when I tested it - without touchend and after removing change was not getting any events fired from a touch device.

@vanrado
Copy link

vanrado commented Aug 7, 2020

Well, the slideend event is not available now :(

I have a little work around that seems to work so far... Please don't fire at me if it doesn't for you ;)

<mat-slider min="0"
            max="100"
            step="1"
            (input)="sliderSliding( $event )"
            (change)="sliderChanged( $event )"
            (mouseup)="sliderMouseButtonReleased( $event )"
            [(ngModel)]="value">
</mat-slider>
    public value: number = 0;
    private _slidingValue: number;

    public sliderSliding( args: MatSliderChange ): void {
        this._slidingValue = args.value;
    }

    public sliderMouseButtonReleased( args: MouseEvent ): void {
        if ( this.value === this._slidingValue ) {
            this.sliderChanged( null );
        }
    }

    public sliderChanged( args: any ): void {
        if ( !!args && !!args.source._lastPointerEvent ) {
            return;
        }

        // Your code to process the event here
    }

Thanks! This helped me out a lot, but I noticed that you will still get multiple events fired when using a touch device or if using a keyboard to move the slider.
I found a simpler way of working around the issue is instead of binding to the change event, to bind to mouseup, keyup and touchend to cover keyboard, mouse and touch. If you do this you no longer need the sliderMouseButtonReleased function and can remove the return at the start of sliderChanged e.g.

<mat-slider min="0"
            max="100"
            step="1"
            (input)="sliderSliding( $event )"
            (mouseup)="sliderChanged( $event )"
            (keyup)="sliderChanged( $event )"
            (touchend)="sliderChanged( $event )"
            [(ngModel)]="value">
</mat-slider>
    public value: number = 0;
    private _slidingValue: number;

    public sliderSliding( args: MatSliderChange ): void {
        this._slidingValue = args.value;
    }

    public sliderChanged( args: any ): void {
        // Your code to process the event here
    }

pls

(mouseup)="sliderChanged( $event )"
            (keyup)="sliderChanged( $event )"
            (touchend)="sliderChanged( $event )"

touchend event is not emitted by mat-slider. Did you test that pls?

Appeared to work when I tested it - without touchend and after removing change was not getting any events fired from a touch device.

touchend doesn't work for me. I used panend (from Hammer.js) event instead of it and it work properly now. Thank you for sharing workaround

crisbeto added a commit to crisbeto/material2 that referenced this issue Aug 7, 2020
Currently when the user presses down somewhere on the slider we emit a `change` event, as well as when they release. While we don't emit the same value twice, the timing seems to be unexpected by consumers, because one happens at the begging of the sequence and another one at the end.

These changes make it so that we only emit the change event once the user has released the slider.

Fixes angular#14363.
@crisbeto crisbeto added has pr P2 The issue is important to a large percentage of users, with a workaround and removed P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Aug 7, 2020
@crisbeto crisbeto self-assigned this Aug 7, 2020
@crisbeto
Copy link
Member

crisbeto commented Aug 7, 2020

Bumping to P2 since people seem to continue to run into this. I also submitted a fix at #20240, but I suspect that it might be difficult to get it merged in, because it can break people's tests.

crisbeto added a commit to crisbeto/material2 that referenced this issue Aug 7, 2020
Currently when the user presses down somewhere on the slider we emit a `change` event, as well as when they release. While we don't emit the same value twice, the timing seems to be unexpected by consumers, because one happens at the begging of the sequence and another one at the end.

These changes make it so that we only emit the change event once the user has released the slider.

Fixes angular#14363.
andrewseguin pushed a commit that referenced this issue Aug 14, 2020
Currently when the user presses down somewhere on the slider we emit a `change` event, as well as when they release. While we don't emit the same value twice, the timing seems to be unexpected by consumers, because one happens at the begging of the sequence and another one at the end.

These changes make it so that we only emit the change event once the user has released the slider.

Fixes #14363.

(cherry picked from commit ce72369)
andrewseguin pushed a commit that referenced this issue Aug 14, 2020
Currently when the user presses down somewhere on the slider we emit a `change` event, as well as when they release. While we don't emit the same value twice, the timing seems to be unexpected by consumers, because one happens at the begging of the sequence and another one at the end.

These changes make it so that we only emit the change event once the user has released the slider.

Fixes #14363.
@samworley
Copy link

Thanks for fixing! Workaround no longer required

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 5, 2020
wagnermaciel pushed a commit to wagnermaciel/components that referenced this issue Jan 14, 2021
Currently when the user presses down somewhere on the slider we emit a `change` event, as well as when they release. While we don't emit the same value twice, the timing seems to be unexpected by consumers, because one happens at the begging of the sequence and another one at the end.

These changes make it so that we only emit the change event once the user has released the slider.

Fixes angular#14363.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: material/slider P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Development

Successfully merging a pull request may close this issue.