Gutenberg: Add Goodreads Block

[Aurorum]

Fixes Automattic/wp-calypso#74494
Fixes #7920

Proposed changes:

Adds a block for Goodreads so that users can update to newer themes - it's currently only available as a legacy widget. This block is similar to the existing widget in that it relies on the default Goodreads styles, but I've added a little bit more customisation which Goodreads offers that was never included in the legacy widget. This can be seen in the Settings panel, and it just controls which books display.

Goodreads also offers a Grid widget that was never used in Jetpack. I feel like Gutenberg is perfect at handling multiple styles, so I've added it with this block.

Grid Widget	Default Widget

I'm marking this as a draft because @supernovia mentioned in the original issue that it'd be good to support authors as well as readers. Unfortunately, I don't have an Authors Goodreads account, so I can't yet see how to implement those widgets. I'll do a bit more digging on that, but would be very grateful for any early feedback - it's been a while since I've tried my hand at making a block!

Does this pull request change what data or activity we track or use?

No changes. Regarding the above section, I don't believe that I'm able to test my code on WordPress.com, and I'm not familiar with e2e tests for Jetpack.

Testing instructions:

• You should be able to create a Goodreads block using a profile URL - so for example: https://www.goodreads.com/user/show/1176283-matt-mullenweg
• Verify that it loads properly and that each of the settings in the sidebar works
• Hopefully nothing is too out of place!

Notably, you should also be able to test this with an Author URL. The Goodreads Widget directs users to documentation on how to acquire their user ID because it's different for authors; with this block, it's much simpler and only requires your profile URL. Example author profile: https://www.goodreads.com/author/show/1406384

[Aurorum]

There has got to be a better way of doing this...but no idea what that would be!

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

---

---

Jetpack plugin:

The Jetpack plugin has different release cadences depending on the platform:

• WordPress.com Simple releases happen daily.
• WoA releases happen weekly.
• Releases to self-hosted sites happen monthly. The next release is scheduled for March 5, 2024 (scheduled code freeze on March 4, 2024).

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

[Aurorum]

Although it would still be nice to add the Author widgets, I can't seem to figure out how to get access to them right now, so I'm marking this as ready for review! Nevertheless, I've added some changes so that it's much simpler for authors to use this block anyway.

Whereas the Goodreads widget requires people to read documentation about how to acquire their User ID in case they give us their Author ID, now you can just input your profile URL and the block will handle distinguishing between the two.

[Aurorum]

@jeherve I’m looking at adding tests for this block and I’d appreciate some advice - what does the fixtures folder in each blocks folder actually do? eg. https://github.com/Automattic/jetpack/tree/trunk/projects/plugins/jetpack/extensions/blocks/eventbrite/test/fixtures

Is this necessary for the tests? Also, apologies if I’ve just completely missed it, but is there any documentation which you could point me to on this? Thanks!!

[supernovia]

Awesome, thanks @Aurorum !

[jeherve]

@Aurorum The fixtures represent mock data for each block. That data is used in the runBlockFixtureTests function here, basically ensuring that the block structure outcome you have in the fixtures matches what's actually outputted when you add a block:

jetpack/projects/plugins/jetpack/extensions/shared/test/block-fixtures.js

Lines 13 to 20 in 260c45a

	/**
	* Run block fixture tests.
	*
	* @param {string} blockName - Block name.
	* @param {Array} blocks - Blocks.
	* @param {string} fixturesPath - Fixtures path.
	*/
	export default function runBlockFixtureTests( blockName, blocks, fixturesPath ) {

You can run cd projects/plugins/jetpack && pnpm test-extensions to test this locally.

[Aurorum]

Thanks @jeherve - I think this block is good to go, except for tests which I don't have much experience with.

I'm a little stuck on how to get the block to display when embedded. I've drawn this from the Calendly Block tests, but it's still set as embedding the profile for me:

	test( 'display block', async () => {
		const attributes = {
		      ...defaultAttributes,
		      goodreadsId: '1176283',
		      userInput: 'https://www.goodreads.com/user/show/1176283-matt-mullenweg',
		    };
		render( <GoodreadsEdit { ...{ ...defaultProps, attributes } } /> );

		let iframe;
		await waitFor( () => ( iframe = screen.getByTitle( 'Goodreads' ) ) );

		expect( iframe ).toBeInTheDocument();
	} );

Other than that, I think this is ready for review now.

[jeherve]

Thanks for tackling this! I left a few remarks below:

---

The Grid view doesn't seem to display a grid in the editor, nor on the frontend:

---

Those previews here are quite big, I think we would benefit from buttons instead, to reclaim that sidebar space:

---

I'm a little stuck on how to get the block to display when embedded.

I see you're on the right track with those tests, apart from some formatting issues with the fixtures! Let's run those tests after the updates above, see how things go!

[Aurorum]

Thanks for the review @jeherve! I've addressed the points you've raised.

The Grid view doesn't seem to display a grid in the editor, nor on the frontend

Is the problem the fact that the images aren't aligned? I've taken the styles directly from Goodreads, and "Grid" is the terminology which they use for it, but we could tidy up the styling on our end if that's something which you think would be beneficial. Alternatively, we could rename it.

I mainly see this style as being something useful for sidebars where you might want to showcase a range of books without taking up much space. In such cases, "Grid" is probably just about a fitting description.

Those previews here are quite big, I think we would benefit from buttons instead, to reclaim that sidebar space

Ah, I wanted to ask about that. I agree that the previews are far too big, but I still feel like the BlockStylesSelector component is appropriate here since it's also an embed. Instead, I wonder if the component is broken? I remember it used to look a lot more aesthetically pleasing.

For example, on the Eventbrite block, it now looks like this.

In #14528, I can see that it used to look like this. Was changing this an intentional design choice or the result of a bug?

I have a fondness to the second design, and I think that it'd make this block look much nicer too. It'd also be clearer which image applies to which style. I don't mind changing this to buttons though, and I definitely agree it'd be preferable to what it is now.

I just think that buttons would also add a layer of complexity for those trying to experiment with the two styles, and something like the old BlockStylesSelector design would be preferable and neat.

[jeherve]

Nice work so far!

I ran the different GitHub checks we have for this PR, so you now have some additional feedback about your work. I also left a few notes below.

Is the problem the fact that the images aren't aligned? I've taken the styles directly from Goodreads, and "Grid" is the terminology which they use for it, but we could tidy up the styling on our end if that's something which you think would be beneficial.

Yes, maybe we could benefit from aligning the images so it looks more like a grid?

I agree that the previews are far too big, but I still feel like the BlockStylesSelector component is appropriate here since it's also an embed. Instead, I wonder if the component is broken? I remember it used to look a lot more aesthetically pleasing.

You're right, it does look broken. I believe it got messed up during a recent Gutenberg update.

To solve this issue for now, I wonder if there would be a way to ditch our custom BlockStylesSelector, and instead use Gutenberg's own registerBlockStyle:
https://developer.wordpress.org/block-editor/reference-guides/block-api/block-styles/

[Aurorum]

Hey @jeherve, just looping back to this old one. If you have some time, I've heavily re-written this block. It now:

• Places all the logic inside a hook so that it's easier to read
• Separates the controls from the rest of the block, also for readability
• Includes proper tests for the settings of the block (rather than just making sure anything renders)
• Removes the legacy widget

I still can't register the two options as an official style because of the fact that it needs to be treated like an attribute. But I've resolved this by adding the icons to the Toolbar, like the Related Posts block does.

Overall, I'm hoping that this works and looks better - both within the code and within the Editor. :) Thanks!!

[scruffian]

@scruffian - am I understanding this correctly? Basing my thoughts on this comment, which is consistent with my testing: #14383 (comment)

That sounds right but it's been a long time since I worked on it!

[Aurorum]

Tests should be fixed now

[Aurorum]

Hey @jeherve - just looping back to this old one. I can see that the tests are passing except for the WordPress.com Tests. However, I don’t seem able to access the logs and see why they’re failing.

Are there any changes needed from my end?

[jeherve]

@Aurorum I think we're getting there! Thanks for working on that! Everything works well for me, except for the number of items. Does this work for you? When I refresh the editor, the number resets back to 5.

[Aurorum]

Whoops, that was just me registering the wrong type - not sure how I missed that! Should be fixed now. :)

Thanks for taking another look!

[jeherve]

This looks good. Let's merge that first version!

[jeherve]

Related diff: D138696-code