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

Make ratio property 0-100 (consistent with iron-range-behavior) #194

Merged
merged 1 commit into from
Jun 13, 2017

Conversation

keanulee
Copy link
Contributor

@keanulee keanulee commented Jun 7, 2017

Fixes #177

Before this patch, paper-slider’s ratio property ranges 0-1.0 (https://github.com/PolymerElements/paper-slider/blob/master/paper-slider.html#L537). However, iron-range-behavior and paper-progress uses ratio between 0-100 (https://github.com/PolymerElements/iron-range-behavior/blob/master/iron-range-behavior.html#L117). The timing changes with Polymer 2.0 surfaces this issue, since iron-range-behavior’s _update method sets ratio between 0-100.

This patch fixes this inconsistency by making paper-slider use the 0-100 range as well. However, it is technically a breaking change since the semantics of ratio is now different. There are alternative fixes (e.g. override the _update method in paper-slider), but that would mean this inconsistency would still exist, so I’m proposing this fix instead.

Copy link
Contributor

@frankiefu frankiefu left a comment

Choose a reason for hiding this comment

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

Making it to be consistent with iron-range-behavior makes total sense. Another way to look at this is ratio in paper-slider has always been returning an incorrect value.

@keanulee keanulee merged commit 7abcfff into master Jun 13, 2017
@keanulee keanulee deleted the fix-ratio branch June 13, 2017 19:29
@keanulee keanulee mentioned this pull request Jun 13, 2017
MXEBot pushed a commit to mirror/chromium that referenced this pull request Sep 2, 2017
paper-slider v2.0.2 has a breaking change.
PolymerElements/paper-slider#194
|ratio| property used to return a value in [0, 1], but now it returns a value in
[0, 100].
Making video player to catch up this change by dividing |ratio| property by 100.

Bug: 752283
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Iad994a2dd7f8a6e5b77698a0deec0812c70fb045
Reviewed-on: https://chromium-review.googlesource.com/646153
Reviewed-by: Yoshiki Iguchi <[email protected]>
Commit-Queue: Naoki Fukino <[email protected]>
Cr-Commit-Position: refs/heads/master@{#499126}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2.0: With initial value set, when dragging for first time makes the slider value go to max.
3 participants