Star rating sample using amp-selector and amp-form

[aghassemi]

Should be fairly straight forward to create a star rating sample using amp-selector and amp-form with radio buttons.

@dandv Is this something you can tackle?

[dandv]

@aghassemi A star rating should submit when the user clicks on one of the stars, so on input change. Would amp-selector be of any use inside the form then, vs. just using the form alone as in this sample?

[aghassemi]

@danbri You are right, since you are using radio buttons and :check psuedo selector, amp-selector won't be of any use. I really like the sample you have, very semantic. Couple of suggestions:
1- add tabindex=1 to the labels + :focus wher-ever we have :hover to support keyboard
2- DOM order does not match the visual order (Start 5 is first in DOM but last visually). They should match for a11y.
3-Test with TalkBack and VoiceOver. The top:-9999px; concerns me a bit. Unlike other screen-readers both TalkBack and VoiceOver can't activate elements that are off screen (since they -wrongly- create a synthetic click event on the coordinates of the element).

[dandv]

Agree with the accessibility requirements. Turns out that out of a ton of rating widgets I've evaluated, none got that reverse keyboard order right. After some hacking, I came up with this:

http://jsbin.com/zejeba

Anyway, I'd like to make this into a proper AMP component, with easily customizable options, and one that can be deployed repeatedly on the page without having to manually change the name. I plan to use the same mechanism as the poll sample for preventing duplicate ratings.

Can we reopen the original issue?

[dknecht]

@dandv Looks like arrow are backwards on safari

[aghassemi]

@dandv There is currently lots of discussions within the core team on what components make sense to be part of core and which ones should to be samples (or maybe eventually a separate layer of components outside of core).

There is no hard rule, but leaning toward "if it is possible without JS/AMP runtime, let's keep it out". Maintenance cost is something we need to worry about in core but ease of use for Dev is also a factor. So it is really a balancing act and amp-rating falls right in the grey area.

What are your thoughts on the benefits of having it as a core component? Would they outvalue the cost?

[dandv]

@aghassemi: three lines of reasoning:

1. I see a lot of extended components added for (or by) third parties. Video players are an example. A party interested in their widget being used, will have a stake in maintaining the AMP extended component for it.

2. Implementing star ratings without JavaScript means every publisher has to reinvent the wheel, and almost guaranteed, get it wrong. I've looked at over 50 star rating widgets, and there was no clear winner (accessibility was the most commonly occurring problem.) JavaScript solves that and enables the component I'm building to have a simple, clear, declarative API: <amp-rating action="https://...">, so that a publisher can easily drop it in. As a DA, I've had demand for this from several publishers, since before I/O 2016. Ratings are ubiquitous in many verticals - travel, e-commerce, lifestyle.

3. In particular for this component, I'll be maintaining it since I'm a Googler and part of the AMP DAs team. Also, @amedina would be happy to help.

[aghassemi]

@dandv Thanks for the detailed response. I agree with your points. Let's make it an amp component. I would be happy to review the PR.

[ericlindley-g]

@dandv @aghassemi Is this a good point in development to take this through design review?

[sebastianbenz]

@dandv could you still create a sample for your amp star rating (until we have a proper component)? It's to good to hide it from the public...

[dandv]

I can, I have the client side done, but not having experience with go and Python (another plea for #400?), I keep running into issues with the backend.

$ goapp serve
INFO     2017-02-15 20:18:37,316 devappserver2.py:769] Skipping SDK update check.
INFO     2017-02-15 20:18:37,440 api_server.py:205] Starting API server at: http://localhost:36794
INFO     2017-02-15 20:18:37,781 dispatcher.py:197] Starting module "default" running at: http://localhost:8080
INFO     2017-02-15 20:18:37,781 admin_server.py:116] Starting admin server at: http://localhost:8000
ERROR    2017-02-15 20:18:41,013 http_runtime.py:396] bad runtime process port ['']
panic: open dist/json/related_products.json: no such file or directory

goroutine 1 [running]:
panic(0x8620e0, 0xc820137f20)
	/home/dandv/util/go_appengine/goroot/src/runtime/panic.go:481 +0x3e6
backend.initProducts(0x996ce0, 0x1f)
	backend/product-listing.go:129 +0x95
backend.InitProductListing()
	backend/product-listing.go:77 +0x30
main47059.init.1()
	server.go:36 +0x38
main47059.init()
	server.go:75 +0x59
main.init()
	_go_main.go:16 +0x45

[sebastianbenz]

Does the file dist/json/related_products.json exist? Otherwise you need to run gulp build first.

…

On Wed, Feb 15, 2017 at 8:26 PM Dan Dascalescu ***@***.***> wrote: I can, I have the client side done, but not knowing go (another plea for #400 <#400>?), I keep running into issues with it. $ goapp serve INFO 2017-02-15 20:18:37,316 devappserver2.py:769] Skipping SDK update check. INFO 2017-02-15 20:18:37,440 api_server.py:205] Starting API server at: http://localhost:36794 INFO 2017-02-15 20:18:37,781 dispatcher.py:197] Starting module "default" running at: http://localhost:8080 INFO 2017-02-15 20:18:37,781 admin_server.py:116] Starting admin server at: http://localhost:8000 ERROR 2017-02-15 20:18:41,013 http_runtime.py:396] bad runtime process port [''] panic: open dist/json/related_products.json: no such file or directory goroutine 1 [running]: panic(0x8620e0, 0xc820137f20) /home/dandv/util/go_appengine/goroot/src/runtime/panic.go:481 +0x3e6 backend.initProducts(0x996ce0, 0x1f) backend/product-listing.go:129 +0x95 backend.InitProductListing() backend/product-listing.go:77 +0x30 main47059.init.1() server.go:36 +0x38 main47059.init() server.go:75 +0x59 main.init() _go_main.go:16 +0x45 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#558 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXOOLrqJ0_fVOmhKJU-fMB4DOxkcB0Vks5rc19qgaJpZM4LwE_Z> .

[dandv]

gulp build fixed it. I had just run backend:serve. But what does a missing file have to do with a "bad runtime port"!? Anyway, thanks :)

[camelburrito]

@dandv - i just had a thought, I think it might be easy to build on top <input type="range"> very easily

• It will make the component more semantic
• You might be able to leverage the ability to set a range without any JS event listeners
• It will make accessibility easy

Here is a code pen i found by google search (i would not implement it like how they did) that uses range input for ratings

https://codepen.io/catharsis/pen/vquyj

Let me know what you think !

(https://css-tricks.com/styling-cross-browser-compatible-range-inputs-css/)

[dandv]

Thanks @camelburrito! Agree that a range input would be more semantic, and it would make fractional/fluid stars easier.

Both <input type="range"> approaches that I've found require JavaScript. Have you seen one that doesn't, so it would work as an ABE sample?

[camelburrito]

I think we might need a little JS - let me see if i can hack something quick;

[camelburrito]

Here is the basic JS bin of what is required

http://jsbin.com/wajiqop

This is unfinished

• we can play around with mask opacity, background colors, various transparent images.

But this is fully functional except for presentation.

[camelburrito]

One more good thing is we can put this in a form and submit would work.

[dandv]

@sebastianbenz:

@dandv could you still create a sample for your amp star rating (until we have a proper component)? It's to good to hide it from the public...

Here it is, by popular demand :)