Add Feedback Prompt in Cart & Checkout blocks sidebar

[Aljullu]

Fixes #1300.

I couldn't get it to appear after the Advanced section. We could achieve this with some custom CSS using flexbox, but I'm not happy modifying the Gutenberg sidebar styles because we might break unexpected things now or in the future, so I would prefer another solution if anybody knows it or leaving the current implementation if we consider it's good enough.

Accessibility

• I've tested using only a keyboard (no mouse)
• All text has at least a 4.5 color contrast with its background

Screenshots

How to test the changes in this Pull Request:

1. Create a post with a Cart or Checkout blocks.
2. Verify there is a Feedback? prompt in the sidebar.

[senadir]

I couldn't get it to appear after the Advanced section.

You can use Slot and Fill since the advanced section has a slot to use.
References: https://developer.wordpress.org/block-editor/components/slot-fill/
the slot name is InspectorAdvancedControls, it will show up before the class field and I'm not sure how to flip them without CSS, but there could be a possibility since both of them uses the slot

<Fill name="InspectorAdvancedControls">
    Your content
</Fill>

[nerrad]

We should use could try an approach similar to the custom_css setting so that this feedback panel can be enhanced on our blocks automatically given the attribute listing support for it: https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/hooks/custom-class-name.js - you may even be able to control placement using the priority on the filter.

[nerrad]

Have we verified this is where we want feedback to go?

[Aljullu]

AFAIK it's not decided yet, so I added this link as a placeholder. I was planning to ask @garymurray when he comes back from AFK. For now, I added a @todo comment so we don't forget about it (b88391a).

[Aljullu]

You can use Slot and Fill since the advanced section has a slot to use.
References: https://developer.wordpress.org/block-editor/components/slot-fill/
the slot name is InspectorAdvancedControls, it will show up before the class field and I'm not sure how to flip them without CSS, but there could be a possibility since both of them uses the slot

<Fill name="InspectorAdvancedControls">
    Your content
</Fill>

Thanks for the tip @senadir! I was looking at it but it seems to add it inside the Advanced panel, right? We would like to have it at the root level but I couldn't find any slot fill for it (at least, not here).

We should use an approach similar to the custom_css setting so that this feedback panel can be enhanced on our blocks automatically given the attribute listing support for it: https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/hooks/custom-class-name.js

I converted it to a HOC (718d3f2), and refactored it to use a filter (96bba02). I tried to copy the custom class inspector implementation as close as possible, but couldn't rely on the supports attribute because we need a specific copy for each block, so feedback is welcome.

[nerrad]

This is shaping up nicely! I have some comments 👇

[nerrad]

This is a neat approach and I ❤️ what you've done here with the HOC and how it's implemented using the filter. The only thing I'm wary about is having all the customization for feedback prompt in this one file (which is right now just content). It'd be more discoverable and easier to work with if individual blocks defined their specific elements. What about one of these options?

Option 1: specific named props for feedback prompt content and link.

In this option, edit components for blocks would be fed specific props that get picked up automatically by this HOC. They'd need to be namespaced appropriately.

• wcFeedbackContent: For the custom content.
• wcFeedbackUrl: For the url where people submit their feedback.

Option 2: use React.Context

This would be similar to option 2 but would utilize react context instead and the withFeedbackPrompt hoc would just read the values from the context. Edit blocks would then just explicitly wrap anything receiving feedback with the feedback prompt provider:

edit

<FeedbackProvider value={ feedBackConfig }>
    <EditComponent>
</FeedbackProvider>

withFeedbackPrompt:

const withFeedbackPrompt = createHigherOrderComponent( ( BlockEdit ) => {
	return ( props ) => {
        const { content, url } = useFeedbackContext();
        if ( content ) { 
            /** do stuff **/

On the surface it seems like the context would be a bit more work, but the advantage is that the EditComponent or anything else in its composition doesn't have to care or worry about passing through any of the props and I like the explicitness in using the provider which makes it clear that this is a component with a feedback element. I wonder if the Provider itself could take care of adding the editor.BlockEdit filter so everything is contained?

[Aljullu]

I was playing with this idea but I'm not sure it's possible. The editor.BlockEdit is applied on an upper level than the block edit function is called, so wrapping the <Edit> block with a context provider, doesn't make that context available when calling the editor.BlockEdit filter.

I pushed a WIP for the checkout block here: 21f889b. Something similar would happen with props (option 1): props passed in the block edit function are not available in the editor.BlockEdit.

Please, let me know if I'm missing something!

---

To solve the issue you raised:

The only thing I'm wary about is having all the customization for feedback prompt in this one file (which is right now just content). It'd be more discoverable and easier to work with if individual blocks defined their specific elements.

If the options above don't work, I thought about some alternative solutions:

• If we don't use the filter but rely only on the HOC like in 718d3f2, we would have access to props/context defined in the edit function.
• We could alternatively create a blockFeedbackRegistry similar to the blocksRegistry we currently have in place. That would allow us to call a registerBlockFeedback() function right after registerBlockType() and have access to the blockFeedbackRegistry inside the editor.BlockEdit filter.

Thoughts?

[nerrad]

Ya you're right the suggestions I have won't work because of how the filter implements the component outside the tree (illustrating one of the flaws of filters!).

I like the blockFeedbackRegistry idea, but I wonder how discoverable that api is, plus it'd be nice to avoid the filter because it feels fragile. Also the filter callback (the HOC) will get invoked for every block.

So I like the direction you are going with the hoc, but we could still use the context and have the hoc read from the context. That way we're not polluting the props unnecessarily for the block and can control where the feedback prompt appears in the inspector controls via the hoc:

withFeedbackPrompt:

/**
 * Adds a feedback prompt to the editor sidebar.
 *
 * @param {WPComponent} BlockEdit Original component.
 *
 * @return {WPComponent} Wrapped component.
 */
const withFeedbackPrompt = createHigherOrderComponent( ( BlockEdit ) => {
	return ( props ) => {
		const { content } = useFeedbackFormContext();
		if ( content ) {
			return (
				<Fragment>
					<BlockEdit { ...props } />
					<InspectorControls>
						<FeedbackPrompt text={ content } />
					</InspectorControls>
				</Fragment>
			);
		}

		return <BlockEdit { ...props } />;
	};
}, 'withFeedbackPrompt' );

Then wrap the Edit component for the block in withFeedbackPrompt.

Then in the block registration you could have something like this:

edit: ( props ) => {
		const feedbackContent = __(
			'We are currently working on improving our checkout and providing merchants with tools and options to customize their checkout to their stores needs.',
			'woo-gutenberg-products-block'
		);
		return (
			<FeedbackFormProvider content={ feedbackContent }>
				<BlockEdit { ...props } />
			</FeedbackFormProvider>
		);
	},

Alternatively, you could just curry the withFeedbackPrompt hoc so it receives the content and url in the curried function returning the HOC itself. So your implementation in edit would be something like:

export default withFeedbackPrompt( __('content for feedback prompt'), 'https://urltofeedback.com' )( Edit );

I'd be happy with either alternative but I think the currying is probably the most straightforward (and less code). Context feels like it may be overkill?

[nerrad]

Oh I also forgot to mention in my review, you need to export the new hoc from the hocs entry point (index.js) along with the other hocs. I couldn't see the new feedback prompt until that was done.

[Aljullu]

Oh I also forgot to mention in my review, you need to export the new hoc from the hocs entry point (index.js) along with the other hocs. I couldn't see the new feedback prompt until that was done.

My bad, I probably removed it while cleaning the code before pushing. 🤦‍♂️ Fixed in 3bd62dd.

[nerrad]

I thought I had submitted my review I did yesterday but I didn't... sorry about that, here it is.

[nerrad]

Ya you're right the suggestions I have won't work because of how the filter implements the component outside the tree (illustrating one of the flaws of filters!).

I like the blockFeedbackRegistry idea, but I wonder how discoverable that api is, plus it'd be nice to avoid the filter because it feels fragile. Also the filter callback (the HOC) will get invoked for every block.

So I like the direction you are going with the hoc, but we could still use the context and have the hoc read from the context. That way we're not polluting the props unnecessarily for the block and can control where the feedback prompt appears in the inspector controls via the hoc:

withFeedbackPrompt:

/**
 * Adds a feedback prompt to the editor sidebar.
 *
 * @param {WPComponent} BlockEdit Original component.
 *
 * @return {WPComponent} Wrapped component.
 */
const withFeedbackPrompt = createHigherOrderComponent( ( BlockEdit ) => {
	return ( props ) => {
		const { content } = useFeedbackFormContext();
		if ( content ) {
			return (
				<Fragment>
					<BlockEdit { ...props } />
					<InspectorControls>
						<FeedbackPrompt text={ content } />
					</InspectorControls>
				</Fragment>
			);
		}

		return <BlockEdit { ...props } />;
	};
}, 'withFeedbackPrompt' );

Then wrap the Edit component for the block in withFeedbackPrompt.

Then in the block registration you could have something like this:

edit: ( props ) => {
		const feedbackContent = __(
			'We are currently working on improving our checkout and providing merchants with tools and options to customize their checkout to their stores needs.',
			'woo-gutenberg-products-block'
		);
		return (
			<FeedbackFormProvider content={ feedbackContent }>
				<BlockEdit { ...props } />
			</FeedbackFormProvider>
		);
	},

Alternatively, you could just curry the withFeedbackPrompt hoc so it receives the content and url in the curried function returning the HOC itself. So your implementation in edit would be something like:

export default withFeedbackPrompt( __('content for feedback prompt'), 'https://urltofeedback.com' )( Edit );

I'd be happy with either alternative but I think the currying is probably the most straightforward (and less code). Context feels like it may be overkill?

[Aljullu]

Thanks for all the good ideas in this thread @nerrad! I updated this PR according to your feedback. Feel free to take another look.

so it receives the content and url

For now, I left the URL hardcoded into the <FeedbackPrompt> component until we know if the feedback URL is going to be shared by all blocks or each one will have a different one.

[nerrad]

Love it! Just one fix needed and it's good to go (hence the pre-approval).

[garymurray]

We can now link this to the Ideas board - I have created a category for WooCommerce Blocks - this link should pre-filter it: https://ideas.woocommerce.com/forums/133476-woocommerce?category_id=384565

We can then also use that link for any future/past blocks that we want to capture feedback on.