Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

introduce SlotFill for Sidebar #3361

Merged
merged 16 commits into from Jan 11, 2021
Merged

introduce SlotFill for Sidebar #3361

merged 16 commits into from Jan 11, 2021

Conversation

senadir
Copy link
Member

@senadir senadir commented Nov 3, 2020

This PR Implements a Slot that is rendered in both Cart and Checkout, just after Totals section.

This slot wrap each fill in an error boundary, so that if a plugin code breaks, it would not affect the block or any other extension hooking into that slot.

Implmentation

import { ExperimentalOrderMeta } from '@woocommerce/blocks-checkout';

const MyApp = () => {
  return (
    <ExperimentalOrderMeta>My stuff here</ExperimentalOrderMeta />
  );
}

registerPlugin( 'my-app', { render: MyApp } );

Closes #3246

Testing instructions

  • The easiest way to test this is to checkout this branch 3940-gh-woocommerce/woocommerce-subscriptions
  • In Cart or Checkout, you should see This is my default export
  • Now to test error boundary, in the above plugin assets/src/js/index.js or anywhere in our plugin, add this code
const myRender = () => {
	return (
		<ExperimentalOrderMeta>
			<div className="block-div-class">This is my other export</div>
		</ExperimentalOrderMeta>
	);
};

registerPlugin( 'woocommerce-two-three', {
	render: myRender,
} );
  • Modify the original code so it throws a runtime error.
const Bug = () => {
	throw Error( 'bug' );
};
const render = () => {
	const [ showBug, setShowBug ] = useState( false );
	return (
		<ExperimentalOrderMeta>
			<div
				className="block-div-class"
				onClick={ () => setShowBug( true ) }
			>
				This is my default export
			</div>
			{ showBug && <Byg /> }
		</ExperimentalOrderMeta>
	);
};
  • On cart or Checkout, click on the text to trigger the error.
  • Only that component is removed from the page, the rest should still be fine

@senadir senadir added category: extensibility Work involving adding or updating extensibility. Useful to combine with other scopes impacted. block: cart Issues related to the cart block. block: checkout Issues related to the checkout block. labels Nov 3, 2020
@senadir senadir requested a review from a team as a code owner November 3, 2020 18:32
@senadir senadir requested review from mikejolley and removed request for a team November 3, 2020 18:32
@senadir senadir self-assigned this Nov 3, 2020
Comment on lines 24 to 26
<BlockErrorBoundary showErrorMessage={ false } key={ i }>
{ fill }
</BlockErrorBoundary>
Copy link
Member Author

Choose a reason for hiding this comment

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

The goal is that this would catch an error but disregard the message and simply not render it.
We can extend this to only show the error if the current user is an admin.

Copy link
Member

@mikejolley mikejolley left a comment

Choose a reason for hiding this comment

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

This implementation looks correct but I wanted to ask if we need to be mindful of slot placement with regards to the slots name. i.e. this one is called OrderReview, but where around the order review is it? Before? After? Within?

@nerrad
Copy link
Contributor

nerrad commented Nov 16, 2020

Besides Mike's feedback (which I agree with) should we make this an ExperimentalOrderReview component until we verify that it works in whatever implementation is exposed for plugins?

Also, should we add tests for this?

@senadir
Copy link
Member Author

senadir commented Nov 16, 2020

I'm having second thoughts about this and I'm not sure yet if we're going to use this or not for the sidebar.
Taking a look at the latest designs that David shared with me, we would need to add 3-5 new slots in the sidebar alone.
This doesn't seem to be very scalable.
A possible approach here might be some kind of filter, that exposes abstracted core components while allowing 3PD to insert new components.

I have a lot of considerations for this approach, but the goal is to expose the least amount of implementation detail as possible while still giving the flexibility to reorder and insert new things.

addFilter('woocommerce/blocks/sidebar', ( Sidebar ) => {
	return (
		<Sidebar>
			<MyView />
			<Sidebar.ShippingSummary />
			<UseShippingForAll />
			<Sidebar.ShippingSelector />
			<Sidebar.Coupon />
			<Sidebar.Taxes />
			<SubscriptionRecurringTotals />
			<Sidebar.Totals />
			<Sidebar.Button />
		</Sidebar>
	);
})

However, I'm not sure how will this solution cascade to several plugins, each doing its own ordering that relies on the fact that something comes after Taxes or Totals.

If not this, we might need to expose preComponent and postComponent for each component above (so like 12 slotFill).

This is just an exploration of the problem and I might post it in a P2 for more in-depth dusscestion.

@nerrad
Copy link
Contributor

nerrad commented Nov 16, 2020

Sounds good, I moved back to the In Progress pipeline.

@senadir
Copy link
Member Author

senadir commented Dec 2, 2020

I'm revisiting this and addressing some feedback.
I'm going to hold on adding tests until we finalize if we want to continue with this approach or not.

@senadir
Copy link
Member Author

senadir commented Dec 2, 2020

Updated this to be named ExprimentalOrderMeta, we can figure out other positions as we move.

@senadir senadir changed the base branch from trunk to add/register-plugin-to-cart-checkout December 2, 2020 16:05
@senadir senadir force-pushed the add/register-plugin-to-cart-checkout branch from 1af5a09 to 29225d7 Compare December 2, 2020 16:07
@woocommerce woocommerce deleted a comment from github-actions bot Dec 2, 2020
@senadir senadir force-pushed the add/register-plugin-to-cart-checkout branch from 29225d7 to 2570cb0 Compare December 2, 2020 16:36
Base automatically changed from add/register-plugin-to-cart-checkout to trunk December 4, 2020 09:52
/**
* External dependencies
*/
import { createSlotFill } from 'wordpress-components';
Copy link
Member Author

Choose a reason for hiding this comment

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

Now that plugin area is merged, we can test this.
I suspect it won't work because of this custom wordpress-components therefore we end up with a different context.
I will try to do some testing.

Copy link
Member Author

Choose a reason for hiding this comment

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

So I tested this and I was right, it won't work unless we export SlotFillProvider from our repo.

@senadir senadir marked this pull request as draft December 4, 2020 10:03
@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2021

Size Change: +2.46 kB (0%)

Total Size: 1.18 MB

Filename Size Change
build/active-filters-frontend.js 8.74 kB +1 B (0%)
build/active-filters.js 8.93 kB -1 B (0%)
build/all-products-frontend.js 34.7 kB +1 B (0%)
build/all-products.js 36.6 kB -2 B (0%)
build/all-reviews.js 9.88 kB +1 B (0%)
build/atomic-block-components/add-to-cart-frontend.js 9.16 kB +1 B (0%)
build/atomic-block-components/add-to-cart.js 7.68 kB -1 B (0%)
build/atomic-block-components/price-frontend.js 2.29 kB -2 B (0%)
build/attribute-filter-frontend.js 18.2 kB -1 B (0%)
build/blocks.js 3.49 kB +1 B (0%)
build/cart-frontend.js 77.3 kB +29 B (0%)
build/cart.js 41 kB +39 B (0%)
build/checkout-frontend.js 92.5 kB +25 B (0%)
build/checkout.js 43.3 kB +50 B (0%)
build/featured-category.js 7.82 kB +2 B (0%)
build/price-filter-frontend.js 15 kB +4 B (0%)
build/product-best-sellers.js 7.57 kB +1 B (0%)
build/product-categories.js 3.24 kB +2 B (0%)
build/product-category.js 8.51 kB +1 B (0%)
build/product-new.js 7.74 kB +1 B (0%)
build/product-on-sale.js 8.13 kB +1 B (0%)
build/product-search.js 3.57 kB -1 B (0%)
build/product-tag.js 6.57 kB +2 B (0%)
build/product-top-rated.js 7.71 kB +1 B (0%)
build/products-by-attribute.js 8.49 kB +1 B (0%)
build/reviews-by-category.js 11.9 kB +1 B (0%)
build/reviews-by-product.js 13.5 kB +1 B (0%)
build/single-product-frontend.js 37.9 kB -2 B (0%)
build/vendors.js 440 kB +6 B (0%)
build/wc-blocks-checkout.js 0 B -9.65 kB (removed) 🏆
build/wc-blocks-data.js 6.91 kB -1 B (0%)
build/wc-blocks-registry.js 2.39 kB +1 B (0%)
build/wc-settings.js 2.4 kB +4 B (0%)
build/wc-shared-context.js 1.53 kB +1 B (0%)
build/blocks-checkout.js 11.9 kB +11.9 kB (new file) 🆕
ℹ️ View Unchanged
Filename Size Change
build/atomic-block-components/add-to-cart--atomic-block-components/button.js 3.29 kB 0 B
build/atomic-block-components/add-to-cart--atomic-block-components/image--atomic-block-components/title.js 334 B 0 B
build/atomic-block-components/button-frontend.js 2.31 kB 0 B
build/atomic-block-components/button.js 839 B 0 B
build/atomic-block-components/category-list-frontend.js 470 B 0 B
build/atomic-block-components/category-list.js 475 B 0 B
build/atomic-block-components/image-frontend.js 1.68 kB 0 B
build/atomic-block-components/image.js 1.13 kB 0 B
build/atomic-block-components/price.js 2.32 kB 0 B
build/atomic-block-components/rating-frontend.js 522 B 0 B
build/atomic-block-components/rating.js 530 B 0 B
build/atomic-block-components/sale-badge-frontend.js 858 B 0 B
build/atomic-block-components/sale-badge.js 865 B 0 B
build/atomic-block-components/sku-frontend.js 388 B 0 B
build/atomic-block-components/sku.js 393 B 0 B
build/atomic-block-components/stock-indicator-frontend.js 568 B 0 B
build/atomic-block-components/stock-indicator.js 572 B 0 B
build/atomic-block-components/summary-frontend.js 917 B 0 B
build/atomic-block-components/summary.js 926 B 0 B
build/atomic-block-components/tag-list-frontend.js 465 B 0 B
build/atomic-block-components/tag-list.js 471 B 0 B
build/atomic-block-components/title-frontend.js 1.35 kB 0 B
build/atomic-block-components/title.js 1.21 kB 0 B
build/attribute-filter.js 12.5 kB 0 B
build/editor-rtl.css 14.9 kB 0 B
build/editor.css 14.9 kB 0 B
build/featured-product.js 10.1 kB 0 B
build/handpicked-products.js 7.5 kB 0 B
build/price-filter.js 10.3 kB 0 B
build/reviews-frontend.js 9.51 kB 0 B
build/single-product.js 10.3 kB 0 B
build/style-rtl.css 18.7 kB 0 B
build/style.css 18.7 kB 0 B
build/vendors--atomic-block-components/price-frontend.js 5.73 kB 0 B
build/vendors-style-rtl.css 1.05 kB 0 B
build/vendors-style.css 1.05 kB 0 B
build/wc-blocks-middleware.js 931 B 0 B
build/wc-payment-method-bacs.js 775 B 0 B
build/wc-payment-method-cheque.js 771 B 0 B
build/wc-payment-method-cod.js 866 B 0 B
build/wc-payment-method-paypal.js 813 B 0 B
build/wc-payment-method-stripe.js 12.1 kB 0 B
build/wc-shared-hocs.js 1.68 kB 0 B

compressed-size-action

@senadir senadir marked this pull request as ready for review January 7, 2021 14:18
Copy link
Member

@mikejolley mikejolley left a comment

Choose a reason for hiding this comment

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

Tested using your instructions and all is well. I am approving this and the subscriptions PR 👍🏻

@@ -95,7 +95,7 @@ public static function register_assets() {
}

if ( Package::feature()->is_feature_plugin_build() ) {
$asset_api->register_script( 'wc-blocks-checkout', 'build/wc-blocks-checkout.js', [], false );
$asset_api->register_script( 'wc-blocks-checkout', 'build/blocks-checkout.js', [], false );
Copy link
Member

Choose a reason for hiding this comment

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

wc-blocks-checkout/wc-checkout-block is going to be confusing. Can we be more specific in our naming here somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

we discussed naming here, we're trying to avoid conflict with core checkout script and our block checkout, if you have any idea of a good name we can implement it anytime because we still didn't publish this.

@senadir senadir merged commit befd4cf into trunk Jan 11, 2021
@senadir senadir deleted the add/slot-fill-for-summary branch January 11, 2021 12:12
@budzanowski budzanowski added this to the 4.3.0 milestone Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
block: cart Issues related to the cart block. block: checkout Issues related to the checkout block. category: extensibility Work involving adding or updating extensibility. Useful to combine with other scopes impacted.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integration Tracking (WC Subscriptions): Create SlotFill integration points for SidebarItems
4 participants