Skip to content

Allow custom translate function when slider steps are specified #299

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

Merged
merged 1 commit into from
Mar 30, 2016
Merged

Conversation

nparrish
Copy link

Currently, if the user specifies a steps array, their translate function is ignored. This changes that. When a translate function is specified, it will be passed the step value (the array index), the slider ID, and the slider label.

@codecov-io
Copy link

Current coverage is 100.00%

Merging #299 into master will not affect coverage as of 7ca4b4b

@@            master    #299   diff @@
======================================
  Files            1       1       
  Stmts          699     701     +2
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit            699     701     +2
  Partial          0       0       
  Missed           0       0       

Review entire Coverage Diff as of 7ca4b4b

Powered by Codecov. Updated on successful CI builds.

return this.options.stepsArray[value];
};
if (this.options.translate) {
this.customTrFn = function(value, sliderId, label) {
Copy link
Member

Choose a reason for hiding this comment

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

Why don't you just do:

this.customTrFn = this.options.translate;

?

Copy link
Author

Choose a reason for hiding this comment

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

Good point.

@ValentinH
Copy link
Member

Why do you need to use a translate function while using stepsArray?

@nparrish
Copy link
Author

For each point on the slider, the displayed text should be different depending on whether it is the high end or the low end of the slider.

@ValentinH
Copy link
Member

Thanks for the updates. Could you add a test to complete the coverage? Also, before I can merge, you need to rebase and squash all your commit into a single one.

@nparrish
Copy link
Author

Done. Thanks for the feedback, and let me know if you'd like to see any other changes.

@ValentinH
Copy link
Member

One last thing: can you rebase and squash everything so there is only one commit?

@nparrish
Copy link
Author

Commits squashed!

@ValentinH ValentinH merged commit 2e5f59d into angular-slider:master Mar 30, 2016
@ValentinH
Copy link
Member

Thanks ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants