Skip to content
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

Star rating component #2691

Open
dandv opened this issue Mar 24, 2016 · 23 comments
Open

Star rating component #2691

dandv opened this issue Mar 24, 2016 · 23 comments

Comments

@dandv
Copy link
Contributor

@dandv dandv commented Mar 24, 2016

A star rating AMP HTML component would enable common 1-5 star rating displays, with optional input.

image

Prior art is documented in my comparison of star rating widgets.

@dvoytenko

This comment has been minimized.

Copy link
Collaborator

@dvoytenko dvoytenko commented Mar 24, 2016

@ericlindley-g ericlindley-g added this to the Backlog milestone Mar 24, 2016
@ericlindley-g

This comment has been minimized.

Copy link
Contributor

@ericlindley-g ericlindley-g commented Mar 24, 2016

Thanks for raising—this would be a good thing to do eventually. Slotting it in the backlog for now, but feel free to submit an intent to implement if you'd like to propose an approach.

@ericlindley-g

This comment has been minimized.

Copy link
Contributor

@ericlindley-g ericlindley-g commented Mar 24, 2016

I should mention, though, that until we build out support for forms, the rating component won't be able to handle the actual input & transmission of data. Happy to see implementations for the ratings UI in the short term, but we won't see the full benefit until forms support is implemented.

@dandv

This comment has been minimized.

Copy link
Contributor Author

@dandv dandv commented Apr 22, 2016

Just a note that forms support is tracked at #3343 (formerly #1286).

@dandv

This comment has been minimized.

Copy link
Contributor Author

@dandv dandv commented Nov 3, 2016

Forms were released to production on 2016-10-31. Holding off until these are released as well:

  • #5654 - Use CLIENT_ID inside amp form as hidden input field
  • #4272 - submit form on input change

See also #5540.

@aghassemi aghassemi self-assigned this Jan 27, 2017
@aghassemi

This comment has been minimized.

Copy link
Contributor

@aghassemi aghassemi commented Jan 27, 2017

@aghassemi

This comment has been minimized.

Copy link
Contributor

@aghassemi aghassemi commented Feb 12, 2017

Re-opening since we decided (see ampproject/amp-by-example#558 (comment)) to make this an AMP component.

@dandv I assume you are still willing to take this one. When you have the API you have in mind ready, let's do a quick forma review with the team. (Just a PR with an example usage and/or markdown file for the API should be enough for the review)

@ericlindley-g

@ericlindley-g

This comment has been minimized.

Copy link
Contributor

@ericlindley-g ericlindley-g commented Feb 12, 2017

@aghassemi @dandv FYI, bumped the priority on this up slightly, given how useful it is for a range of e-comm use cases — @dandv, do you have a sense of a timeframe for this component?

@camelburrito

This comment has been minimized.

Copy link
Contributor

@camelburrito camelburrito commented Feb 18, 2017

Repasting my thoughts here

@dandv - i just had a thought, I think it might be easy to build on top 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/)

@jridgewell

This comment has been minimized.

Copy link
Member

@jridgewell jridgewell commented Mar 10, 2017

@camelburrito: Closable?

@camelburrito

This comment has been minimized.

Copy link
Contributor

@camelburrito camelburrito commented Mar 10, 2017

@rudygalfi

This comment has been minimized.

Copy link
Contributor

@rudygalfi rudygalfi commented Mar 15, 2017

Any updates?

@dandv

This comment has been minimized.

Copy link
Contributor Author

@dandv dandv commented Mar 16, 2017

@ericlindley-g ericlindley-g added this to Sprint (candidate) in UI Mar 24, 2017
@ericlindley-g ericlindley-g moved this from Sprint (candidate) to Backlog (shortlist) in UI Mar 24, 2017
@ericlindley-g ericlindley-g moved this from Backlog (shortlist) to Sprint (candidate) in UI Mar 24, 2017
@rudygalfi

This comment has been minimized.

Copy link
Contributor

@rudygalfi rudygalfi commented Mar 27, 2017

Thanks for the last update. Anything new happening with this?

@ericlindley-g ericlindley-g moved this from Sprint (candidate) to Backlog in UI Mar 31, 2017
@dandv

This comment has been minimized.

Copy link
Contributor Author

@dandv dandv commented May 23, 2017

Now that I/O is over, I'm back to work on this since it can unblock a number of partners.

/cc @joemarini FYI

@szynszyliszys

This comment has been minimized.

Copy link

@szynszyliszys szynszyliszys commented Jun 12, 2017

I have fixed the reverse arrow issue in Starability.css. Now it works with keyboard in correct order. You might consider that. https://github.com/LunarLogic/starability

@ampprojectbot

This comment has been minimized.

Copy link
Member

@ampprojectbot ampprojectbot commented Oct 20, 2017

This issue hasn't been updated in awhile. @dandv Do you have any updates?

@aghassemi

This comment has been minimized.

Copy link
Contributor

@aghassemi aghassemi commented Oct 23, 2017

p2 -> p3 since there is an ABE sample for it already.

@nicopujol

This comment has been minimized.

Copy link

@nicopujol nicopujol commented Jul 20, 2019

We'd love to use this feature and know of many users who would too.

@rudygalfi

This comment has been minimized.

Copy link
Contributor

@rudygalfi rudygalfi commented Jul 22, 2019

@nicopujol Have you had a chance to take a look at this guide for implementing star ratings in AMP: https://amp.dev/documentation/examples/interactivity-dynamic-content/star_rating/ ?

It would be great to hear what kinds of use cases you think a dedicated component would address vs what's possible with the capabilities illustrated in the guide linked above.

cc @nainar

@nicopujol

This comment has been minimized.

Copy link

@nicopujol nicopujol commented Jul 23, 2019

Hi @rudygalfi @nainar ,

Some thoughts:

  • We run sites on WordPress (largely food blogs) with the official AMP plugin. Due to site theme, recipe plugin and other CSS elements on the site, we sometimes see CSS overflow on AMP pages, breaking the layout. Not sure if adding more CSS would make things worse.

  • WordPress recipe plugins embed star rating for standard non-AMP pages, and display ratings today in read-only for AMP. We would ask the developers of these plugins to implement support for AMP ratings. To do this, I need to give to give them something long term that will not change soon in terms of coding.

  • When reading the CSS article, it states "stay tuned for the amp-rating component" which suggests it is a stop gap measure until an AMP JS component becomes available.

  • CSS implementation also mentions limitations on non Chrome browsers. We have a large chunk of mobile traffic coming from non-Chrome (mostly iOS/Safari).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.