Automattic / wp-calypso

The JavaScript and API powered WordPress.com
https://developer.wordpress.com
GNU General Public License v2.0
12.4k stars 1.98k forks source link

Theme Showcase: Add a "Trending" tab #54088

Open sixhours opened 3 years ago

sixhours commented 3 years ago

Investigate the feasibility of adding a Trending tab section to the Showcase, as shown in Onur's mockups:

We may already have an endpoint to fetch "trending" themes to the Showcase.

kathrynwp commented 3 years ago

Just noting for the record that "trending" themes aren't something users typically ask for or seem interested in.

Instead, they are interested in finding a theme that fits the vision they have in their mind, or that looks like another site they've seen.

So I'm not sure how valuable this view would be and I'm just wondering what the reasoning behind adding this view would be.

blackjackkent commented 3 years ago

I looked into the API endpoint for this today, and it seems like we have a function which (in its comments at least) seems to indicate that it returns themes in a "trending" order by default:

(link in OpenGrok available on request)

/**
* Returns an array of WP_Theme objects based on the arguments, ready for the
* Themes REST API.
*
* @param array $args {
*     Additional arguments. Optional.
*
*     @type mixed  $allowed True to return only allowed themes for a site.
*                           False to return only disallowed themes for a site.
*                           'site' to return only site-allowed themes.
*                           'network' to return only network-allowed themes.
*                           Null to return all themes.
*                           Defaults to 'network'.
*     @type int    $blog_id The blog ID used to calculate which themes are allowed. Defaults to 0,
*                           synonymous for the current blog.
*     @type string $sort    'trending', 'popular', 'newest', 'name'.
*                           Defaults to 'trending'.
*     @type bool   $retired Include retired themes in the query.
*                           Defaults to false.
*     @type string $locale  The locale to which all texts are translated to, if available.
*                           Defaults to 'en_US'.
*     @type string $currency The currency to retreive the theme price in.
*                           Defaults to 'USD'.
*     @type bool   $extended Include additional meta information.
*                           Defaults to false.
* @return array Array of WP_Theme objects.
*/

This function is not used in the (current) v1.2 themes API endpoint, but is used in v1.1.

I'm not sure yet how to plug this into the existing behavior of the showcase; I will probably work on piggybacking off of Caroline's work in https://github.com/Automattic/wp-calypso/pull/54126 and see if we can include another REST call there if need be.

blackjackkent commented 3 years ago

Backend research results in more detail (crossposted from Slack):

https://public-api.wordpress.com/rest/v1.1/themes

This endpoint (notably, not the most recent version of the themes API) accepts a "sort" param that defaults to "trending" if it's not included in the request. That eventually filters down to the function at wp-content/admin-plugins/theme-viewer/popular-trending-themes.php#200

If I'm honest, I'm not entirely sure what particular brand of magic is being used to calculate the result list here; it calls a number of different functions with a bunch of logic. It looks like it's retrieving info from a database called theme_stats_daily (or pulling from a cached calculation if available) and also makes use of a site option: get_site_option( $this->option_name_popular_with_rank_values );

Assuming this is logic we want to keep, I can definitely update this into a v1.2 endpoint specifically for this purpose - the jury is still out, though, on whether it will be easy to plug a completely separate network call into the way the theme showcase fetches themes. (It looks like our current plan is to prioritize other tabs before worrying about this one.)

blackjackkent commented 3 years ago

(Moving this back out of in progress for now since the consensus seems to be that it's at the bottom of the priority list at the moment.)

kwight commented 3 years ago

This function is not used in the (current) v1.2 themes API endpoint, but is used in v1.1.

The way the API is built, new versions of an endpoint will include anything from previous versions not overridden (through inheritance) – so as long as v1.2 isn't overriding the v1.1 trending functionality, it will be available in v1.2 also.

blackjackkent commented 3 years ago

Spent some time working on this today. As I discussed with Kirk yesterday, the 1.2 API themes API specifically uses a different theme fetching mechanism than the 1.1 API (the latter of which is the one which specifically had a documented 'trending' sort option). I did some digging today on the 1.2 API to see if its query mechanism had a sort option buried in the code but not specifically documented, but it doesn't seem to. Since both are available to us, however, using the v1.1 API for this purpose seems equally viable. So I'm now working on plugging a call to that endpoint into the tabs rendering code.