Testimonial Block #11723

marekhrabe · 2019-03-27T15:30:08Z

Changes proposed in this Pull Request:

This PR adds a new block for presenting Testimonials. Its basic structure is:

testimonial body
author name
author title
author image

Testing instructions:

Proposed changelog entry for your changes:

Add Testimonial Block

TODOs

simison · 2019-03-27T21:50:36Z

extensions/blocks/testimonial/edit.js

+/**
+ * Internal dependencies
+ */
+import { __ } from '../../utils/i18n';


Just wanted to give heads up that we'll remove the custom i18n wrapper soon, in which point this can just be changed to @wordpress/i18n import. :-)

Aight, #11743 is merged so this can be rewritten to:

import { __ } from '@wordpress/i18n'; __( 'My testimonial', 'jetpack' ); // note textdomain!

marekhrabe · 2019-04-01T08:47:13Z

Progress: I have working rich text fields for testimonial, author and title (but not for the image yet) and I've implemented very rough style variants (just approximations).

marekhrabe · 2019-04-01T12:53:56Z

Alignment control very basically working + I've added flexbox so there are first signs of layout resembling the design.

marekhrabe · 2019-04-01T13:02:26Z

Every field is optional so it's easy to create this:

but it still shows up fully when you have the block selected:

getdave

So great to see this coming to life 👍 A few minor points I noticed.

getdave · 2019-04-03T10:12:22Z

extensions/blocks/testimonial/edit.js

+		return (
+			<Fragment>
+				<BlockControls>
+					<AlignmentToolbar value={ attributes.align } onChange={ this.onChangeAlign } />


Do you need this or can you just set align in the supports definition?

I've tried supports: { align: [ … ] } but it automatically applies some styles to the block for left/right which made it float around other blocks (like an image block would). I need the block to stay where it is and only use the alignment attr to restyle things inside. I couldn't get it with out of box alignment so I made my own implementation. Would be great to hear from somebody else whether it makes sense.

supports.align is block aligment and for content alignment you'd need something custom as you've done here. I think even core Paragraph is custom. Maybe some day core will have supports.contentAlignment or similar :-)

https://wordpress.org/gutenberg/handbook/designers-developers/developers/block-api/block-registration/#supports-optional

getdave · 2019-04-03T10:14:14Z

extensions/blocks/testimonial/editor.scss

+.wp-block-jetpack-testimonial__meta {
+    display: flex;
+    flex-direction: column;
+    color: #999;


Should you use a color var here?

It's definitely a placeholder. I think I'll need to run this by design folks whether to use something like opacity so it works universally across themes and dark/light variants or whether there is a variable to use. There definitely are vars in editor but we also need to figure out the color for the frontend and that might be different between themes.

getdave · 2019-04-03T10:14:24Z

extensions/blocks/testimonial/editor.scss

+    display: flex;
+    flex-direction: column;
+    color: #999;
+    font-size: 80%;


Should you use ems here?

Another placeholder, I don't even know the exact styles we will use yet so I just put random percentages.

getdave · 2019-04-03T10:16:05Z

extensions/blocks/testimonial/index.js

+	description: __( 'Lets you display testimonials from customers and clients.' ),
+	icon: 'embed-photo',
+	category: 'jetpack',
+	keywords: [ __( 'testimonial' ) ],


You shouldn't need to duplicate this if it's the name of the Block itself? Some useful alternative keywords could be

https://www.merriam-webster.com/thesaurus/testimonial

getdave · 2019-04-03T10:16:39Z

extensions/blocks/testimonial/index.js

+		{ name: 'normal', label: _x( 'Normal', 'block style' ) },
+	],
+	attributes: {
+		align: {


Can this not be handled by supports? Or is it more custom?

getdave · 2019-04-03T10:17:47Z

extensions/blocks/testimonial/save.js

+			className="wp-block-jetpack-testimonial__content"
+			value={ content }
+		/>
+		{ mediaId && mediaUrl && <img src={ mediaUrl } alt="" /> }


The alt attribute is currently blank. Is this intentional? It feels like it should have a valid value as the only time an empty alt is valid is when the image is always guaranteed to be purely presentational in nature.

I'm thinking about using the authors name || title || "" as the alt. Does it make sense?

…thout the media too

marekhrabe · 2019-04-11T05:51:50Z

The block now looks much more like a final product in terms of the editing experience.

The CSS for layout is much more robust, works responsively and handles that all fields are optional.
The block looks the same in both editor and frontend.
When selected in the editor, unfilled fields will show up so they can be edited.
We now have a properly sized and positioned placeholder for the avatar that can be clicked to add/change the avatar.
Block toolbar contains an edit button to change the avatar, as well as a button to remove it completely.
Left/center/right alignments work in all different styles.
You can transform the core Quote and Pullquote blocks into this new Testimonial and vice versa.
Keyboard navigation between fields works with arrow keys and Tab. Avatar can be focused via Tab and modal opened with Space or Enter key.

What we are lacking now is a proper CSS for all those different styles and some other related questions. I'll pull those from Figma and convert absolute px values into ems for font-sizes, line-heights and paddings.

What colors should we use? The prototype contains some specific grays but we should rather inherit the theme colors and lighten the author name and title using opacity. Sounds good?

I'm still conflicted on the decision to make three of same styles, differentiating only in font size. One of the even uses italics users cannot opt out of, even though in all other styles they can control whether their text is in italics or not via the formatting toolbar. As per me, I would prefer either removing the size options at this form completely or bring back the size picker into the sidebar for all styles.

thomasguillot · 2019-04-11T08:08:38Z

What we are lacking now is a proper CSS for all those different styles and some other related questions. I'll pull those from Figma and convert absolute px values into ems for font-sizes, line-heights and paddings.

Have a look at the Quote Block for the CSS.
The "Quote" style should look like the "Standard" style of the Quote Block and then the other 2 styles can just use similar font-sizes, line-heights and paddings.

What colors should we use? The prototype contains some specific grays but we should rather inherit the theme colors and lighten the author name and title using opacity. Sounds good?

Yep, sounds good 👍

I'm still conflicted on the decision to make three of same styles, differentiating only in font size. One of the even uses italics users cannot opt out of, even though in all other styles they can control whether their text is in italics or not via the formatting toolbar. As per me, I would prefer either removing the size options at this form completely or bring back the size picker into the sidebar for all styles.

Having 3 fairly similar styles doesn't make sense. Originally, I received the feedback that a font-size setting was too much complexity for this block but it was before looking at block location and homepage template.
So, could we:

Add a new setting section in the sidebar https://cloudup.com/c5FmfOpAXqC
Remove "Large" and "Small"
Rename "Normal" to "Standard"

simison · 2019-04-15T05:56:04Z

extensions/blocks/testimonial/testimonial.php

+/**
+ * Testimonial Block.
+ *
+ * @since 7.1.0


Remember to bump this to 7.3

simison · 2019-04-15T05:59:19Z

extensions/blocks/testimonial/editor.js

+/**
+ * Internal dependencies
+ */
+import registerJetpackBlock from '../../utils/register-jetpack-block';


Please rebase; as of #11971 this has been moved to shared from utils.

simison · 2019-04-15T05:59:53Z

extensions/setup/index.json

@@ -20,6 +20,7 @@
  ],
  "beta": [
    "seo",
+    "testimonial",


Please rebase; as of #11971 this file has been moved to extensions/index.json from setup.

mtias · 2019-04-15T10:33:28Z

It seems to me this block is trying to do too much on its own and so it ends up replicating some interactions through somewhat ad hoc means — like using "styles" to change the position of elements.

Have you thought about using inner blocks to assemble a block with children instead?

<InnerBlocks
	template={ [
		[ 'core/image', { style: 'round' } ], // the profile picture
		[ 'core/quote', { style: 'large' } ], // the quote and name
	] }
/>

<InnerBlocks
	template={ [
		[ 'core/image', { style: 'round' } ],       // the profile picture
		[ 'core/heading' ],                         // the person's name
		[ 'core/paragraph', { style: 'large' } ],   // the quote text
	] }
/>

The block itself could have some attributes like style variations or background (or just integrate the new "group" block) but the main advantage is that the user can move the picture to go below the quote, etc, and you don't have to replicate functionality that is already provided through those blocks (font sizing, etc).

You can lock the template so that new blocks cannot be inserted.

marekhrabe · 2019-04-16T07:23:34Z

I agree with your ideas in general but looking at the block (and its design iterations), I don't think our editing capabilities are there yet to just take other core blocks and make a composite. Sure, your code drafts would work but they simplify the block to the point where we can just build the content in the editor, not using any extra block. The devil is in details here and I'll take the multi-axis centering and optional fields as an example:

This block has a "footer" with the author name, title and avatar. All those fields are optional and when you focus you'll see "fill-the-blanks" template like this:

On the frontend (and also in the editor when not selected), we make sure the content is always centered perfectly based on data that are filled in.

The center alignment shows this the best:

Just a name - simply centered

Name and title - again, just centered.

Name, title and avatar - this time text fields are left-centered, next to the avatar and the whole thing is in center.

Just the name and avatar - same as the case above and now the name is also vertically centered against the avatar.

In the right alignment we flip the order of image and text fields, text-alignment, margins etc…

I hope that makes a point that this block has some capabilities that are not (easily) possible with just core blocks. With what is available today, I've chosen to implement a custom block for this purpose.

You seem to suggest that moving the avatar doesn't fit into the "Style" category. Is that somewhere defined? All docs I've found about styles are purely technical and explain the mechanics and that they are used to "provide alternative styles to block".

mtias · 2019-04-16T17:39:08Z

I hope that makes a point that this block has some capabilities that are not (easily) possible with just core blocks.

Is that even if you use the block wrapper to control the specific styles, etc? .wp-block-testimonial .wp-block-paragraph {}

All docs I've found about styles are purely technical and explain the mechanics and that they are used to "provide alternative styles to block".

There are no established patterns for this yet. Moving elements is something that is learned through the block movers, so it's expected to apply to elements of the block. A good example might be the fields of the Jeptack Contact Form block.

Even if you create your own block, it might be good to have custom child blocks for the different parts to benefit from further UI clarity (movers, drag and drop, etc).

marekhrabe · 2019-04-17T08:50:55Z

I don't see how to achieve that. There are additional wrapping elements needed, not just those holding the content, in order for the centering to work.

The save() generates an HTML with this shape:

div.wp-block-jetpack-testimonial
  div.wp-block-jetpack-testimonial__content
    p
    p
    p
  div.wp-block-jetpack-testimonial__author
    img.wp-block-jetpack-testimonial__media
    div.wp-block-jetpack-testimonial__meta
      div.wp-block-jetpack-testimonial__name
      div.wp-block-jetpack-testimonial__title

With InnerBlocks, I can only make paragraphs of content, media, name and title elements as immediate siblings (correct?) which doesn't allow for the appropriate layout to be coded.

mtias · 2019-04-29T18:15:00Z

@marekhrabe with InnerBlocks you can have your own custom blocks, which will let you have any wrapper you want. What child blocks afford you in that case is more granular control over the experience, since each "element" can have its own controls. Does that make sense?

getdave · 2019-05-01T09:30:53Z

Thanks to both @marekhrabe and @mtias for the productive discussion.

Having reviewed the comments my understanding is that the suggested approach is to:

Mirror the JP contact form approach of using InnerBlocks to provide the Block with increased flexibility in terms of Design variations and user control
Create bespoke sub-components to ensure we can retain whatever custom DOM we need to create the Design styles via CSS (we can use the allowedBlocks on InnerBlocks to lock the components that are allowed)

We may also consider exploring a CoBlocks style approach to the Design variations. Variations that switching out the InnerBlocks template aren't well suited to the Styles UI. We should consider using an approach similar to the CoBlocks row Block where you select a template upfront. This may end up being a phase 2.0 for this Block.

If we're going down this route then we'll need agreement from Marek. As the person who has authored the Block so far, I feel we need to give his concerns due credence (I'm not suggesting for one minute that this hasn't happened so far, but merely reinforcing that he has raised these for a good reason).

Perhaps we could explore a proof of concept using Matias's suggested approach? If this looks like it it's heading in the right direction we can then embark on a full refactor. I'd rather we don't commit to this upfront and then find ourselves having to back out at considerable effort.

Thanks again to Marek for all his work on bringing the Block to its current state. Great work which I'm sure we can build on going forward!

stale · 2019-07-30T09:59:38Z

This PR has been marked as stale. This happened because:

It has been inactive in the past 3 months.
It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this PR has clear testing instructions, is it up to date with master, and it is still valid. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

simison · 2019-07-30T10:02:23Z

cc @mkaz in case Tinker is interested in this. ;-)

stale · 2019-10-28T10:18:03Z

This PR has been marked as stale. This happened because:

It has been inactive in the past 3 months.
It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this PR has clear testing instructions, is it up to date with master, and it is still valid. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

initial scaffolding

929baf0

marekhrabe added the [Status] In Progress label Mar 27, 2019

jeherve added [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Block] Testimonial [Type] Feature Request labels Mar 27, 2019

simison reviewed Mar 27, 2019

View reviewed changes

added fields and very basic styles

5cb0afd

matticbot added the Touches WP.com Files label Apr 1, 2019

make things richer $

a7f7260

marekhrabe force-pushed the add/testimonial-block branch from c3c5afd to a7f7260 Compare April 1, 2019 08:49

added flexbox, basic alignment setup

fe32726

more styling

d1499d4

getdave reviewed Apr 3, 2019

View reviewed changes

marekhrabe added 8 commits April 5, 2019 15:11

new save(), shared scss between frontend and editor

0f3d896

renamed file

2fd44f2

transforms from/to quote and pullquote

9c49dcf

add proper classname for image

7384aa6

padding between author image and meta

7b1fd1b

move meta paddings into avatar margins - that way everything works wi…

ab1a65a

…thout the media too

add UI for avatar edit/removal

c0c02a8

update keyboard logic

c497575

matticbot removed the Touches WP.com Files label Apr 11, 2019

marekhrabe added 3 commits April 11, 2019 17:50

make name & title fields full-width in editor

667c630

square ratio images

20c4792

add classname for media presence

3b899ba

marekhrabe added 5 commits April 15, 2019 13:10

centering logic

4b8bdd9

fix margin

196a7ee

only add padding in spotlight when image present

939b461

remove deprecated styles, rename normal to standard

7505419

basic font size picker implementation

ddef525

simison reviewed Apr 15, 2019

View reviewed changes

extensions/blocks/testimonial/testimonial.php

/**

* Testimonial Block.

*

* @since 7.1.0

Copy link

Member

simison Apr 15, 2019

Choose a reason for hiding this comment

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

Remember to bump this to 7.3

simison reviewed Apr 15, 2019

View reviewed changes

stale bot added the [Status] Stale label Jul 30, 2019

stale bot removed the [Status] Stale label Jul 30, 2019

stale bot added the [Status] Stale label Oct 28, 2019

marekhrabe closed this Mar 23, 2020

marekhrabe deleted the add/testimonial-block branch March 23, 2020 10:15

kraftbj removed the [Status] In Progress label Oct 8, 2021

anomiex removed the [Status] Stale label Jul 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Testimonial Block #11723

Testimonial Block #11723

marekhrabe commented Mar 27, 2019 •

edited

Loading

simison Mar 27, 2019 •

edited

Loading

simison Apr 2, 2019

marekhrabe commented Apr 1, 2019

marekhrabe commented Apr 1, 2019

marekhrabe commented Apr 1, 2019

getdave left a comment

getdave Apr 3, 2019

marekhrabe Apr 3, 2019

simison Apr 15, 2019

getdave Apr 3, 2019

marekhrabe Apr 3, 2019

getdave Apr 3, 2019

marekhrabe Apr 3, 2019

getdave Apr 3, 2019

getdave Apr 3, 2019

getdave Apr 3, 2019

marekhrabe Apr 3, 2019

marekhrabe commented Apr 11, 2019 •

edited

Loading

thomasguillot commented Apr 11, 2019

simison Apr 15, 2019

simison Apr 15, 2019

simison Apr 15, 2019

mtias commented Apr 15, 2019 •

edited

Loading

marekhrabe commented Apr 16, 2019

mtias commented Apr 16, 2019 •

edited

Loading

marekhrabe commented Apr 17, 2019

mtias commented Apr 29, 2019

getdave commented May 1, 2019

stale bot commented Jul 30, 2019

simison commented Jul 30, 2019

stale bot commented Oct 28, 2019

Testimonial Block #11723

Testimonial Block #11723

Conversation

marekhrabe commented Mar 27, 2019 • edited Loading

Changes proposed in this Pull Request:

Testing instructions:

Proposed changelog entry for your changes:

TODOs

simison Mar 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marekhrabe commented Apr 1, 2019

marekhrabe commented Apr 1, 2019

marekhrabe commented Apr 1, 2019

getdave left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marekhrabe commented Apr 11, 2019 • edited Loading

thomasguillot commented Apr 11, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtias commented Apr 15, 2019 • edited Loading

marekhrabe commented Apr 16, 2019

mtias commented Apr 16, 2019 • edited Loading

marekhrabe commented Apr 17, 2019

mtias commented Apr 29, 2019

getdave commented May 1, 2019

stale bot commented Jul 30, 2019

simison commented Jul 30, 2019

stale bot commented Oct 28, 2019

marekhrabe commented Mar 27, 2019 •

edited

Loading

simison Mar 27, 2019 •

edited

Loading

marekhrabe commented Apr 11, 2019 •

edited

Loading

mtias commented Apr 15, 2019 •

edited

Loading

mtias commented Apr 16, 2019 •

edited

Loading