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

Move and adjust ratings-bars and ratings-stars. #633

Merged
merged 23 commits into from
Jul 24, 2024
Merged

Conversation

StevenDufresne
Copy link
Contributor

@StevenDufresne StevenDufresne commented Jun 24, 2024

We have ratings components on wp.org in there places:

This PR moves blocks added to https://github.com/WordPress/wporg-theme-directory to this project for use across the site.

Approach

I decided to use block.json to expose the attributes and custom filters for themes to use to populate the values. I'm not 100% sure it's the best approach. I considered using render_block_data instead of a custom filter but thought this was more maintainable in the future and we use custom filters for other blocks. I can be convinced otherwise.

Example Usage

wporg/ratings-stars

<!-- wp:wporg/ratings-stars /-->

Attributes

Attribute Type
rating number

wporg/ratings-bars

<!-- wp:wporg/ratings-bars /-->

Attributes

Attribute Type Description
ratingsCount number The total of ratings, must match sum of all values in ratings
ratings array(5) Rating count. The array must have 5 items. [1] = 1 star, [5] = 5 star.
slug string The slug of the theme or plugin.
supportUrl string Base url for support forums.

Filter Example

use function WordPressdotorg\Theme\Theme_Directory_2024\{get_support_url, get_theme_information};

/**
 * Update ratings blocks with real rating data.
 *
 * @param array $data    Rating data.
 * @param int   $post_id Current post.
 *
 * @return array
 */
function set_rating_data( $data, $post_id ) {
	$theme = get_theme_information( $post_id );
	if ( ! $theme ) {
		return $data;
	}

	return array(
		'rating' => $theme->rating,
		'ratingsCount' => $theme->num_ratings,
		'ratings' => $theme->ratings,
		'supportUrl' => get_support_url( $theme->slug . '/reviews/' ),
	);
}
add_filter( 'wporg_ratings_data', __NAMESPACE__ . '\set_rating_data', 10, 2 );

Testing this PR

You can test this PR in conjunction with WordPress/wporg-theme-directory#146.

That environment doesn't include the ratings table by default so it make makes sense to test in a sandbox.

  1. Load this PR in your theme directory environment on try/refactor-rating-starstry/refactor-rating-stars
  2. Navigate to a theme, the reviews should be present

@StevenDufresne StevenDufresne marked this pull request as ready for review June 25, 2024 02:39
@StevenDufresne StevenDufresne requested a review from ryelle June 25, 2024 02:55
Copy link
Contributor

@ryelle ryelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The blocks do work, but I have some feedback on the filter process. In the query total & query filter blocks, the filter runs over a placeholder value so the filter only applies to that item, I think an approach like that would be better here.

mu-plugins/blocks/ratings-bars/render.php Outdated Show resolved Hide resolved
mu-plugins/blocks/ratings-bars/src/block.json Show resolved Hide resolved
mu-plugins/blocks/ratings-bars/src/block.json Outdated Show resolved Hide resolved
mu-plugins/blocks/ratings-stars/render.php Outdated Show resolved Hide resolved
mu-plugins/blocks/ratings-stars/index.php Outdated Show resolved Hide resolved
Copy link
Contributor

@ryelle ryelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now, one last little 🧹 cleanup request before merging — the attributes can be removed from block.json.

@ryelle ryelle merged commit bd22c7c into trunk Jul 24, 2024
2 checks passed
@ryelle ryelle deleted the try/rating_blocks branch July 24, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants