Make WordPress Core

Opened 12 years ago

Closed 12 years ago

#28147 closed defect (bug) (fixed)

No ratings string never used?

Reported by: pavelevap Owned by: SergeyBiryukov
Priority: normal Milestone: 4.2
Component: Themes Version: 3.9
Severity: normal Keywords: good-first-bug has-patch dev-feedback
Cc: Focuses:

Description

String "No ratings." is probably never used.

https://core.trac.wordpress.org/browser/tags/3.9/src/wp-admin/theme-install.php#L236

There is following condition:

<# if ( data.num_ratings ) { #>

But data.num_ratings is the whole string "(based on %s rating)" and never empty or null. When there is no rating, data.num_ratings will be "(based on 0 rating)". And this is not ideal...

I am not sure which variable contains real number of ratings to patch this issue.

Attachments (6)

28147.diff (556 bytes ) - added by valendesigns 12 years ago.
28147.2.diff (516 bytes ) - added by kevdotbadger 12 years ago.
28147.3.diff (1.4 KB ) - added by kevdotbadger 12 years ago.
Screenshot 2015-01-07 12.52.53.png (515.7 KB ) - added by kevdotbadger 12 years ago.
Screenshot 2015-01-07 12.52.50.png (585.2 KB ) - added by kevdotbadger 12 years ago.
28147.4.diff (1.4 KB ) - added by valendesigns 12 years ago.

Download all attachments as: .zip

Change History (14)

#1 @jorbin
12 years ago

  • Component I18NThemes
  • Keywords good-first-bug added

wp_ajax_query_themes will need to be modified to include the raw count of ratings and we will need to update the check to look for that.

#2 @SergeyBiryukov
12 years ago

  • Keywords needs-patch added
  • Milestone Awaiting Review4.2

@valendesigns
12 years ago

#3 @valendesigns
12 years ago

  • Keywords has-patch added; needs-patch removed

There's a simpler solution. I've added patch 28147.diff that fixes this issue without any major changes.

#4 @kevdotbadger
12 years ago

Actually, i think there's an even easier way, which is what I think the original developer intended.

data.num_ratings is always a string, which is the obvious problem. jorbin mentioned amending some code to return the raw count, this actually exists in data.rating (as an int), this is what should be used.

28147.2.diff fixes this.

#5 @kevdotbadger
12 years ago

  • Keywords dev-feedback added

This actually got me thinking. Should the star's actually show if there's no rating? They are always going to blank, and it doesn't really add anything to the page. In fact, in my opinion, it looks negative as you could easily interpret the theme as rubbish purely because there's no rating.

28147.3.diff rewords the text that is displayed based on the amount to reviews, see the screenshots.

Anyone disagree or have a better idea?

Version 1, edited 12 years ago by kevdotbadger (previous) (next) (diff)

#6 @SergeyBiryukov
12 years ago

28147.3.diff looks good to me. (A small note, we don't generally escape apostrophes with slashes like that, the preferred way is to switch to double quotes, see the coding standards.)

I'd suggest changing the string to "This theme has not been rated yet".

Last edited 12 years ago by SergeyBiryukov (previous) (diff)

#7 @valendesigns
12 years ago

The refreshed 28147.4.diff patch does not use contracted english grammar and is created from the repository root instead of inside src.

#8 @SergeyBiryukov
12 years ago

  • Owner set to SergeyBiryukov
  • Resolutionfixed
  • Status newclosed

In 31072:

Theme install: Display an appropriate string if theme has not been rated yet.

props kevdotbadger, valendesigns.
fixes #28147.

Note: See TracTickets for help on using tickets.