[#536] Fix ex.Utils.clamp min/max handling #673
Conversation
Fices excaliburjs#536 - Update ex.Utils.clamp
jedeen
added
the
enhancement
eonarheim
changed the title
kamranayub
changed the title
|
Also, I'm thinking to be good citizens we should ensure this function has proper tests around it. |
There was a problem hiding this comment.
Thanks for working on this! A couple of additional notes:
There is a small typo in the commit message. In additon, we would appreciate it if all commit messages followed our styleguide.
Please update CHANGELOG.md with the changes you made in this pull request. You can find more information in our pull request guidelines.
|
Roger. |
Bring branch to current
|
@jedeen @FerociousQuasar Let's not worry about the commit messages on this one since we'll squash the messages. Any issues @excaliburjs/core-contributors? LGTM |
| it('can clamp a number to a maximum and minimum', () => { | ||
| expect(ex.Util.clamp(0, 10, 20)).toBe(10); | ||
| expect(ex.Util.clamp(15, 10, 20)).toBe(15); | ||
| expect(ex.Util.clamp(30, 10, 20)).toBe(20); |
There was a problem hiding this comment.
Could you add another couple tests for Infinity and -Infinity to test those boundaries?
| @@ -104,7 +104,7 @@ module ex.Util { | |||
| } | |||
|
|
|||
| export function clamp(val, min, max) { | |||
There was a problem hiding this comment.
Could you add the number types to each parameter to make sure people don't accidentally pass in other types?
Closes #536
Changes: