Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

feat(rating): Added rounding logic to rating value #3415

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
6 changes: 6 additions & 0 deletions src/rating/rating.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ angular.module('ui.bootstrap.rating', [])
this.render = function() {
$scope.value = ngModelCtrl.$viewValue;
};

$scope.$watch('value', function(value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be an unnecessary & unperformant approach.

A better approach would be to create a formatter, that way it only gets run when the model value changes.

Copy link
Author

Choose a reason for hiding this comment

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

I'll take a look

if (typeof value !== 'undefined' && value % 1 !== 0) {
$scope.value = Math.round(value);
}
});
}])

.directive('rating', function() {
Expand Down
14 changes: 14 additions & 0 deletions src/rating/test/rating.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,20 @@ describe('rating directive', function () {
});

it('changes the number of selected icons when value changes', function() {
$rootScope.rate = 2.1;
$rootScope.$digest();

expect(getState()).toEqual([true, true, false, false, false]);
expect(element.attr('aria-valuenow')).toBe('2');

$rootScope.rate = 2.5;
$rootScope.$digest();

expect(getState()).toEqual([true, true, true, false, false]);
expect(element.attr('aria-valuenow')).toBe('3');
});

it('rounds off the number of stars shown with decimal values', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you meant for this to be the title of the new test ^ instead of changing the name of an old test.

$rootScope.rate = 2;
$rootScope.$digest();

Expand Down