Skip to content

fix(slider): correct click/slide event position #1303

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix(slider): correct click/slide event position #1303

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/lib/slider/slider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ export class MdSlider implements AfterContentInit, ControlValueAccessor {

this.isActive = true;
this.isSliding = false;
this._sliderDimensions = this._renderer.getSliderDimensions();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is only necessary in one handler, I would just leave it in the click handler. Also needs a comment explaining why the dimensions need to be recalculated here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jelbourn, two things:

  • to make the unit tests pass, _sliderDimensions needs to be updated in both onClick and onSlideStart, though it may be because the tests generate the click/slide events explicitly.
  • initially I couldn't write a failing test, I believe it is due to the way the click/slide events are generated in the tests.

The click/slide events in the unit tests take a "percentage", grab the up-to-date dimensions of the element and calculate the event coordinates based on that, which is backwards to how the browser does it. This means the values are always "correct" even if you move the sliders' parent around. See: https://github.com/angular/material2/blob/master/src/lib/slider/slider.spec.ts#L688

Adjusting these methods to take (pre-calculated) coordinates of an event, tests fail/pass as expected. Am I missing something obvious though...?

this._renderer.addFocus();
this.updateValueFromPosition(event.clientX);
this.snapThumbToValue();
Expand All @@ -207,6 +208,7 @@ export class MdSlider implements AfterContentInit, ControlValueAccessor {
event.preventDefault();
this.isSliding = true;
this.isActive = true;
this._sliderDimensions = this._renderer.getSliderDimensions();
this._renderer.addFocus();
this.updateValueFromPosition(event.center.x);
}
Expand Down