-
Notifications
You must be signed in to change notification settings - Fork 95
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
Recipe ratings #342
base: master
Are you sure you want to change the base?
Recipe ratings #342
Conversation
First thing to decide is if we want to use the Rating type for the recipe rating or just a simple field. I would opt for the Rating type, because it is relatively simple but still has about everything necessary. bestRating would be 10 and worstRating 1, 0 is reserved for no rating. We can have a set of default ratingExplanations for when this field is left empty, but it's also good to give an option for a free text expression (such as "Great with macaroni and cheese!"). For the front-end, I would like to know if everyone is okay with using a third-party icon set? I assume we would use stars for the ratings and I have almost a copy-paste solution for a pretty nifty UI using the free FontAwesome icons. Using unicode characters is one option, of course, but controlling the look of the UI would be near impossible because it would depend on on the character set installed on the user's machine. |
Regarding the type decision: I favor the Regarding the icon set, I am ok with you solution as long as the usage is free for open source. |
@sam-19, just to be clear: What exactly is needed from the backend perspective? It would maybe be a good first step if we define the interface (together). If I find a few free minutes, I might be able to implement the backend part soon. This might help you with a functional (hopefully) backend to test the frontend. |
Sorry, I'm really busy at the moment, I'll try to get working on this as soon as possible. The backend would of course have to save the ratings with the recipes, preferably in the recipe JSON file. When a recipe is loaded (for viewing or editing) the numerical value of the rating (ratingValue) and possible short description (ratingExplanation) should be returned. So the API could inject an attribute like this into the recipe data The schema.org Rating includes also the field author, and this may be a bit tricky to handle on the backend. So to remain faithful to the original proposal, each contentRating field should in fact be an array of Rating objects, one for each person who have rated the recipe. First problem arises, if the recipe is shared but some people only have read access. Should they not be able to rate? Should their ratings be saved in a database entry? And what if someone wants their rating to remain private, even if the recipe is shared (this would we impossible if the recipes are saved as plain text on the file system)? I think some compromise will be necessary. Maybe a good place to start would be to only pass each user's own ratings through the API and maybe implement something with a combined or average rating later. Also, if we want ratings to be displayed on the index and/or search page recipe listing, those objects need to at least include the ratingValue. |
OK, this might get a bit lengthy, sorry for that. I try to organize this a bit and separate issues here. Where should the ratings be stored?I think the main credo for this app is to save everything into the JSON files as the DB is considered to break more easily. Thus, I suggest that the JSON file is going to be changed upon each recipe rating. I think we need to define a new API endpoint for CRUD operation of the user's (single) rating of a (single) recipe. I suggest to locate these under @sam-19 if you do not mind, I will just define the routes accordingly in the Sharing of recipes and ratingsCurrently the only way to share recipes is by sharing the whole folder of recipes. Until these changes are carried out, I suggest to stick with a simple sharing mechanism as well for the ratings. Each user can rate a recipe at most once (update is of course allowed but only one rating entity). The user can be identified by Any change in the ratings causes the backend to recalculate the aggregated ratings in the JSON. That way, it needs to be calculated only once. Viewing of the ratings of the recipeWhen it comes to presenting the recipe to the user, the current state of using the JSON as a transport medium can be kept. The ratings are to be embedded into the structure at appropriate keys. So showing of the data should be straight forward by just extending the vue components. Adding other views and searchingAnother issue is sorting by recipes (at least rating of x) and filtering etc. Here I suggest to keep the effort at minimum for now. The list of recipes is delivered with a list of recipes as far as I know (I have not checked at the moment). Thus, all information should be available at the corresponding places. If something is missing we can add more data or more API endpoints on demand. |
I wanted to suggest to migrate the API endpoints to versioned ones. So instead of |
This all sounds good, @christianlupus! A versioned API pretty much is mandatory for RESTful implementations, if anyone ever wants to extend this application that way, and not a bad idea even if that never happens. I was wondering if we could get away with using a resource instead of separate routes for the rating, i.e. "/api/v1/ratings"? That would require that you can catch those requests somewhere along the chain in the backend and substitute the rating ID with the recipe ID. PS: I'm not ignoring this PR or your review requests, just need a couple more days to handle some urgent matters. |
OK, I will update that accordingly soon. The next days I am a bit short in time but I will move towards versioned API endpoints.
My main intention was to use a special API and reject any changes made by Or do you mean to use a service and reinterpret the id in the service endpoint as recipeId instead of the ratingId (merely against the basic idea of a service to be a shortcut for a certain set of routes with an id as identifier)?
I see. No issue here for me. I just did not want to wilder in your main topic. |
The current state I did not test yet but it should work. I wanted to avoid to do the migration involved as this will potentially involve more work to keep the dev environment ready for other tests. |
Signed-off-by: sam-19 <[email protected]>
Signed-off-by: Christian Wolf <[email protected]>
Signed-off-by: Christian Wolf <[email protected]>
Signed-off-by: Christian Wolf <[email protected]>
Signed-off-by: Christian Wolf <[email protected]>
Signed-off-by: Christian Wolf <[email protected]>
… file Signed-off-by: Christian Wolf <[email protected]>
Signed-off-by: Christian Wolf <[email protected]>
Signed-off-by: Christian Wolf <[email protected]>
Signed-off-by: Christian Wolf <[email protected]>
@sam-19 is this something you can help get tested and or finished? @christianlupus If sam is not coming back what would be left to get this some what functional? User based and Versioned rest URIs? Thanks, An interested party member |
@patn03 I think the next steps would be to define the UI interface (where should be visible what). Starting from there the UX must be implemented and any missing parts in the API can be identified. After implementing these, we are good to go to have a test. |
Hey, and sorry I haven't been able to contribute in a while. There are a couple of other projects that take up practically all of my available time at the moment. I started this PR because I already had an almost identical component working in another Vue project. To save some time, I will give some hints on how this can be implemented in a simple and robust way (minimal javascript). It uses two icons, one for a solid star shape and one for an outline shape. Additionally, if we want to use half stars (and I suggest we should) it also uses a solid half-star and and outline of a half-star. Before I started this PR I checked that at least FontAwesome should have the required icons available in their free package. Below is the Vue component template part. My example uses the font-awesome-icon component but it can just as easily be made using the < i > tags. <div class="rating">
<div class="rating-stars">
<label v-for="(v,i) in 11" :key="'hl'+i" @dblclick="clearRating()">
<input v-model="rating"
:class="'rating'+i"
type="radio" name="rating"
:value="i" @click="i%2 ? null : halfStar(i)"
/>
<font-awesome-icon v-for="n in (i-i%2)"
:key="'fs'+n"
:icon="[n%2 ? 'fas' : 'fal','star']"
:class="n%2 ? 'star-fill' : 'star-outline'"
/>
<font-awesome-icon v-if="i%2" :icon="['fas','star-half']" class="star-fill" />
<font-awesome-icon v-if="i%2" :icon="['fal','star-half']" class="star-outline" />
</label>
<div>
<font-awesome-icon v-for="n in 5" :key="'bg'+n" icon="star" />
</div>
</div>
<input type="text" class="rating-desc" :placeholder="$t('general.rating_descriptions')[rating]" v-model="ratingDesc" />
</div> Idea being that we first add a radio input with 10 options that we bind the rating property to, then on top of them display the correct amount of stars (depending on which option is selected, this is done using CSS below). The last for-loop creates 5 light gray stars in the background to show in place of the "missing" stars. Clicking the last full star will remove half of the last star (e.g. if the rating is 3 stars, clicking the third star will make it 2,5 stars) and double clicking any star will reset the rating to zero. After the star icons there is a text field for an optional description. In the methods part of the component: clearRating: function () {
this.rating = 0
},
halfStar: function (r) {
// Turn full star into half star
if (this.rating === r) {
this.rating -= 1
}
}, The CSS part: .rating {
position: relative;
}
.rating-stars {
display: inline-block;
position: relative;
}
.rating input[type=text] {
/* This is the optional description field */
position: absolute;
left: 5.5em;
width: whatever
}
/* Ignore clicks on icons and radio inputs (leaving only the labels to interact with) */
.rating-stars svg,
.rating-stars label input[type=radio] {
pointer-events: none
}
/* Position the first label and first child div (gray background stars) */
.rating-stars label {
position: absolute;
top: 0;
left: 0;
height: 1em;
cursor: pointer;
}
.rating-stars div {
position: absolute;
top: 0;
left: 0;
width: 5.1em;
height: 1em
}
/* Layer the consecutive labels on top of each other */
/* Even numbers are full stars */
.rating-stars label:nth-child(2) {
z-index: 9;
width: 1.1em
}
.rating-stars label:nth-child(4) {
z-index: 7;
width: 2.1em
}
.rating-stars label:nth-child(6) {
z-index: 5;
width: 3.1em
}
.rating-stars label:nth-child(8) {
z-index: 3;
width: 4.1em
}
.rating-stars label:nth-child(10) {
z-index: 1;
width: 5.1em
}
/* Odd numbers are half stars */
.rating-stars label:nth-child(3) {
z-index: 10;
width: 1.1em
}
.rating-stars label:nth-child(5) {
z-index: 8;
width: 2.1em
}
.rating-stars label:nth-child(7) {
z-index: 6;
width: 3.1em
}
.rating-stars label:nth-child(9) {
z-index: 4;
width: 4.1em
}
.rating-stars label:nth-child(11) {
z-index: 2;
width: 5.1em
}
/* Hide the radio inputs */
.rating-stars label input {
position: absolute;
top: 0;
left: 0;
opacity: 0;
}
/* Full stars must cover their underlying half-stars */
.rating-stars label input.fullstar {
z-index: 2
}
/* Make the icons invisible by default */
.rating-stars label svg {
position: absolute;
color: transparent;
}
/* Background stars should be visible */
.rating-stars div svg {
color: #000000; /* or some other color */
opacity: 0.2;
}
/* Position the stars inside their containers */
/* Each container has stars up to the rating it represents */
.rating-stars label svg:nth-child(2),
.rating-stars label svg:nth-child(3),
.rating-stars div svg:nth-child(1) {
left: 0
}
.rating-stars label svg:nth-child(4),
.rating-stars label svg:nth-child(5),
.rating-stars div svg:nth-child(2) {
left: 1em
}
.rating-stars label svg:nth-child(6),
.rating-stars label svg:nth-child(7),
.rating-stars div svg:nth-child(3) {
left: 2em
}
.rating-stars label svg:nth-child(8),
.rating-stars label svg:nth-child(9),
.rating-stars div svg:nth-child(4) {
left: 3em
}
.rating-stars label svg:nth-child(10),
.rating-stars label svg:nth-child(11),
.rating-stars div svg:nth-child(5) {
left: 4em
}
/* Finally, make only the stars inside the selected/hovered label visible */
.rating-stars:hover label:hover input ~ svg.star-fill,
.rating-stars:not(:hover) label input:checked ~ svg.star-fill {
color: some-star-color
}
/* Make the star outline a bit darker */
.rating-stars:hover label:hover input ~ svg.star-outline,
.rating-stars:not(:hover) label input:checked ~ svg.star-outline {
color: #000000;
opacity: 0.2; /* or some other opacity */
} This was made for another project and may not translate flawlessly, but it should already get you pretty far. |
Here's a pull request for a recipe rating system (issue #323). This will be a collaborative effort, as work is needed both in the front-end and the back-end.