search

fyneworks / star-rating

jQuery Star Rating Plugin
MIT License
29 stars 17 forks source link

Add a "stars" option #10

Closed dkniffin closed 9 years ago

dkniffin commented 10 years ago

With most jQuery UI widgets, you simply give a container element, and some options, and it will create all the necessary DOM elements. This is not quite possible with this plugin.

What I propose is the addition of a "stars" option, which indicates how many stars to display. Then, if the radios aren't already created, it would create them. This allows all the options for the widget to be defined all in one place (javascript, rather than html and javascript)

fyneworks commented 10 years ago

How is this for a start? https://github.com/fyneworks/star-rating/commit/f93559776e454e09d3b9c1377c8e5c7599c36542

If the element context is not an input and the options.stars is available (better validation might be required, ie.: is it actually a number? range? etc) then the plugin will insert the form elements and re-invoke itself on behalf of the new elements. This was none of the rest of the plugin needs to change.

All options from the initial call are passed on, so.... $(...).rating({stars:5,callback:function(value){ alert(value) }}) will show an alert with the selected value and $(...).rating({stars:10,split:2}) will create 5 split stars (10 possible values to select)

Also accepts a "values" array to determine the values of the newly created elements. It defaults to 1, 2, 3, 4, 5, etc, but $(...).rating({stars:5, values:['A','B','C','D','E']}); will use those values instead.

SO.... summary of new options:

fyneworks commented 10 years ago

ps.: I foresee some issues regarding the context of the control. The plugin determines which <input type="radio"/> belong together based on their context. This is a combination of which <form> they're in and the input's/widget's name. If the input is not within a <form> the context is assumed to be the <body>.

That will be a problem for users who want to create instances of the plugin regardless of forms and don't supply unique control names.

For example, running this code will create a new widget on the fly: $('<div/>').rating({stars:7}).appendTo('body') But if you run it again it will append 7 stars to the previous control: $('<div/>').rating({stars:7}).appendTo('body')

Whereas, running this code will create a new widget on the fly: $('<div/>').rating({stars:7,name:'control-1'}).appendTo('body') But if you run this code a new control will be created with the name "control-2". $('<div/>').rating({stars:7,name:'control-2'}).appendTo('body')

In an ideal world, should the plugin will take there of this?

fyneworks commented 10 years ago

Actually, had a think about it and this should do the trick: https://github.com/fyneworks/star-rating/commit/39d5010d853014e5f4254fc340cb39ee792755f3

fyneworks commented 10 years ago

Testing looks OK. I'll leave you to have a look. https://www.fyneworks.com/jquery/star-rating/test/unpacked.html

Gotta run now and won't be available for the weekend but would love some help bringing this plugin up to speed (specially with regards to documentation - some small improvements will go a long way to helping out the community), that is, if you don't mind helping out!

dkniffin commented 10 years ago

Wow. Thanks for the quick turnaround on this. I'll download the new version and test it out in my application. I'll let you know what I find.

I would definitely be willing to add the additional options. I'll fork the issue10 branch, then submit a pull request.

fyneworks commented 10 years ago

Thank YOU for helping out. This plugin is now 6 years old and I still use it myself, frequently. It's well overdue a face-lift but I just hadn't had the time. Appreciate the input and more than happy to chip in with a few improvements here and there. All hail open source :+1: