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

[Rating] Add titles for each stars #3621

Closed
wants to merge 8 commits into from
Closed

[Rating] Add titles for each stars #3621

wants to merge 8 commits into from

Conversation

lepit31
Copy link
Contributor

@lepit31 lepit31 commented May 5, 2015

Add an attribute "titles" to set a custom title for each star by it position on array.
if the index of the star is greater than the titles array length return the index +1 of the star
Add a default value ["bad","poor","regular","good","gorgeous"]

@karianna
Copy link
Contributor

karianna commented May 5, 2015

@lepit31 - The existing tests fail and we'd also like to see some new tests for this please :-).

@lepit31
Copy link
Contributor Author

lepit31 commented May 5, 2015

Fix the compilation bug
Add 5 test for the rating
(5 test are ok with a 'grunt test' on my local repository )

This is my first git commit , sorry for the annoyance.

@rvanbaalen
Copy link
Contributor

No problem man. Appreciate the input. Everybody has got to start somewhere.

@@ -16,6 +16,10 @@ Rating directive that will take care of visualising a star rating bar.
_(Defaults: false)_ :
Prevent user's interaction.

* `titles`
_(Defaults: ["bad","poor","regular","good","gorgeous"])_ :
Copy link
Contributor

Choose a reason for hiding this comment

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

Add spaces after each comma

it('should return the custom title for each star', function() {
element = $compile('<rating ng-model="rate" titles="[44,45,46]"></rating>')($rootScope);
$rootScope.$digest();
expect(getTitles()).toEqual(['44','45','46','4','5']);
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix spacing

@wesleycho
Copy link
Contributor

I spent some time refactoring the work & cleaning up the history just now - I changed the default values to ['one', 'two', 'three', 'four', 'five'], since it is more suitably generic. I will be merging shortly.

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

Successfully merging this pull request may close these issues.

None yet

4 participants