ampproject / amp-by-example

DEPRECATED: AMP by Example has been merged into amp.dev
http://amp.dev
Apache License 2.0
753 stars 505 forks source link

Star Rating carn't select checked="checked" stars #978

Open ghost opened 6 years ago

ghost commented 6 years ago

Hey,

https://ampbyexample.com/advanced/star_rating/preview/ https://ampbyexample.com/playground/#url=https%3A%2F%2Fampbyexample.com%2Fadvanced%2Fstar_rating%2Fsource%2F

It is impossible to vote 2 stars. When your product have a 5 star best rating und 5 stars displayed via checked="checked" no one can rate the product with 5 stars!?!

Best, Daniel

kul3r4 commented 6 years ago

Hi, could you explain this better? I tried to:

Could you list the steps you are doing?

ghost commented 6 years ago

Hi,

The Problem is: You carnt vote the default value checked="checked".

The example has 2 stars by default and when you try to vote the example with 2 stars, it isnt possible.

Best, Daniel

kul3r4 commented 6 years ago

Hi, what is not exactly possible? For me:

What exactly happens after you click the 2 stars? could you send a snapshot?

sparhami commented 6 years ago

When you click on a different star rating, a network request is made (and a message is shown, thanking the user for rating). I think the problem might be that if you click on the current star rating, there is no network request made.

One use case might be to have the current rating (i.e. the average rating, not the rating you gave) as the initial selected value. The problem for that use case would be that you couldn't vote for the current rating.

I think in that case, there might be a better way to show the initial state. Maybe a separate rating component would be useful for this use case.

ghost commented 6 years ago

@sparhami Thats exact my use case. Display the average rating e.g. 5 stars as the initital selected value and none couldn't vote 5 stars.

fstanis commented 6 years ago

Hi all,

I think there may be some confusion about what the initial "checked" 2-star state is supposed to represent in this example: the idea is that this is what a returning user, who'd already voted 2-stars, would see. We aren't currently persisting the rating server-side, so the imaginary scenario is that you'd already previously voted 2-stars and are now returning to change your mind. In this case, making a network request when 2-star is selected again would be redundant.

With that in mind, if you want to use the initial state to display the average rating, the checked attribute is not a good way to go. Instead, you can use a separate class instead, e.g. current-rating and then just slightly modify the CSS to account for this use-case:

+    form#rating:not(.amp-form-submit-success):not(.amp-form-submit-error) .rating:not(:hover) > input.current-rating ~ label:before {
+      content: "★";
+      position: absolute;
+      left: 0;
+      color: red;
+    }
     .rating > *:hover,
     .rating > *:hover ~ label,
     .rating:not(:hover) > input:checked ~ label {
       color: transparent;  /* reveal the contour/white star from the HTML markup */
       cursor: inherit;  /* avoid a cursor transition from arrow/pointer to text selection */
     }

And in the HTML use this class instead of checked="checked":

       <input name="rating" type="radio" id="rating3" value="3" on="change:rating.submit" />
       <label for="rating3" title="3 stars">☆</label>

-      <input name="rating" type="radio" id="rating2" value="2" on="change:rating.submit" checked="checked" />
+      <input name="rating" type="radio" id="rating2" value="2" on="change:rating.submit" class="current-rating" />
       <label for="rating2" title="2 stars">☆</label>

       <input name="rating" type="radio" id="rating1" value="1" on="change:rating.submit" />

This makes the 2-star "clickable", while still providing an indication of what the average rating (2-star) is in a different color before the user submits their rating.

sparhami commented 6 years ago

Clearing a rating is also a valid use case (perhaps more so if you just wanted a simple thumbs up/down), which simple radios don't address. You might also want to partially fill in stars (to distinguish an average rating of something like 4.5 from 5.0 or 4.0).