Bump @wordpress version and use Modal component for shipping label purchase #2336

c-shultz · 2021-02-22T17:24:02Z

Description

Bumps @WordPress components and elements versions up. The updated styling of components v11 improved the look and feel of the Modal component. I ran into some dependency road blocks with using v12, so we can save that update for later.
Replaces Calypo's Dialog component with @WordPress's Modal component for the shipping label purchase modal

This only replaces the component on the shipping label purchase modal. I'll continue the Dialog replacement efforts on future PRs. I decided to stop here because this was already pretty big (especially given the dependency updates).

The shipping summary has also been nested in a Card component because that worked best with the updated header styles.

There are numerous style updates to make this all work well.

Related issue(s)

#2333

Steps to reproduce & screenshots/GIFs

Testing:

Test all JS since the @wordpress update could affect any part of the JS app
Test shipping label purchase modal
- Mobile vs. desktop
- Styling

I took some liberties with the design. I think we should aim to use the replacement components as intended, so I prioritized trying to make things look nice instead of trying to totally replicate the old design/layout. Let me know if there's any big concerns, otherwise we can get some design feedback once more components have been replaced

c-shultz · 2021-02-22T17:24:58Z

assets/stylesheets/style.scss

@@ -250,6 +250,7 @@

 // Just the tooltip WordPress Component styles is all we use for now.
 @import "~@wordpress/base-styles/colors";
+@import "~@wordpress/base-styles/variables";


This has to be manually added here so @WordPress components can find it. I would have thought this would be handled automatically by npm/webpack, but this is also how wc-admin does things.

c-shultz · 2021-02-22T17:25:23Z

...tensions/woocommerce/woocommerce-services/views/shipping-label/label-purchase-modal/index.js

-					<FormSectionHeading>
-						{ translate( 'Create shipping label', 'Create shipping labels', { count: Object.keys( props.form.packages.selected ).length } ) }
-					</FormSectionHeading>
-					<Button className="label-purchase-modal__close-button" onClick={ onClose }>


Button is built into Modal component

c-shultz · 2021-02-22T17:26:20Z

package.json

@@ -27,10 +27,9 @@
  "license": "GPL-2.0",
  "description": "Connect allthethings",
  "devDependencies": {
-    "@slack/web-api": "^5.8.0",
+    "@slack/web-api": "^5.15.0",


Several version got bumped to the latest compatible version (by npm update). I figured this was a good time while we we're tearing everything up anyway 🤷

c-shultz · 2021-02-22T17:27:31Z

package.json

+    "@hot-loader/react-dom": "^16.14.0",
+    "@wordpress/babel-preset-default": "^5.0.1",
+    "@wordpress/base-styles": "^3.3.0",
+    "@wordpress/components": "^12.0.7",


This is a major update of @wordpress/components from 8.5 to 12.0.7

I went back to 11.1.15 because the dependencies are messy on 12 and I can't fix it so far

npm-shrinkwrap.json

bborman22

Functionally everything worked as expected and visually it all matched up well. I only found two issues, one is a mobile visual issue and the other is a React warning.

The mobile visual issue was with the label purchase modal width and padding. The modal didn't go full screen width and it had padding on the sides which squished everything together causing some overlap.

Current WCS:

This branch:

The Reach warning I got was:

Cannot update during an existing state transition (such as within `render`). Render methods should be a pure function of props and state.
    in PackageInfo (created by Localized(PackageInfo))

I looked through the PackageInfo component and I'm pretty certain the issue is when a package is not selected for the order and it updates the props inside the render. Everything still worked as expected with the warning, but was wondering if there was a better way to handle that props update?

woocommerce-services/client/extensions/woocommerce/woocommerce-services/views/shipping-label/label-purchase-modal/packages-step/package-info.js

Line 59 in 40ccf29

props.setPackageType( orderId, siteId, packageId, userMeta.last_box_id );

c-shultz · 2021-02-25T15:08:42Z

client/extensions/woocommerce/woocommerce-services/state/shipping-label/actions.js

@@ -271,6 +274,43 @@ const tryGetLabelRates = ( orderId, siteId, dispatch, getState ) => {
 		} );
 };

+export const setPackageType = ( orderId, siteId, packageId, boxTypeId ) => (


setPackageType is moved up here now so it's defined in time

c-shultz · 2021-02-25T15:09:01Z

Good finds, @bborman22. I tweaked the styles for narrow screens. I think it's as good as the previous version at least.

You found the right line that was triggering that warning. I moved that functionality into actions to trigger when the printing modal opens up.

bborman22

Awesome, that warning is gone now! The modal is looking better, but I am still seeing a gap in the modal on my screen.

I noticed it is coming from this line and the fact that the new modal doesn't have the .dialog.card classes to set width to 100% at that breakpoint. Not sure if we should add those classes or remove that selector and just apply it right to .label-purchase-modal which seemed to work?

woocommerce-services/client/extensions/woocommerce/woocommerce-services/views/shipping-label/label-purchase-modal/style.scss

Lines 7 to 22 in 4666610

    
           &.label-purchase-modal { 
        
           	height: calc(100% - 10vmin); 
        
           	width: calc(100% - 10vmin); 
        
               max-height: 1080px; 
        
               max-width: 1920px; 
        
             	box-shadow: rgba(151,150,150,.5) 2px 2px 7px; 
        
           	background-color: $studio-gray-0; 
        
           	@include breakpoint( '<660px' ) { 
        
           		&.dialog.card { 
        
           			height: 100%; 
        
           			max-height: 100%; 
        
           			width: 100%; 
        
           			max-width: 100%; 
        
           		} 
        
           	}

c-shultz · 2021-02-25T17:37:58Z

Thanks for catching that. I'm so blind to these things sometimes on my own PR. 🙈

The card style applies to the cards within the modal and that seems to be working OK. I put a 100% width style in for the modal on narrow screens. Much better now 😄

bborman22

Looks good! I think this is good to go now!

c-shultz added 3 commits February 20, 2021 21:37

Replace modal component for shipping label printing.

e7f7951

Update @WordPress dependencies to get latest components/styles.

931f573

Bump to latest @worpdres/components.

36a7f9b

c-shultz commented Feb 22, 2021

View reviewed changes

c-shultz added 2 commits February 22, 2021 11:35

Go back to @wordpress/components@11.1.15

d7ab053

Improve shipping label purchae modal styling.

40ccf29

c-shultz changed the title ~~WiP -- Replace modal component~~ Bump @wordpress version and use Modal component for shipping label purchase Feb 22, 2021

c-shultz marked this pull request as ready for review February 22, 2021 23:03

bborman22 self-requested a review February 23, 2021 14:21

bborman22 requested changes Feb 23, 2021

View reviewed changes

c-shultz added 3 commits February 23, 2021 14:06

Remove padding on narrow screens.

93064e4

Move dispatch out of render.

189f82a

Make more testable and add tests.

4666610

c-shultz commented Feb 25, 2021

View reviewed changes

bborman22 reviewed Feb 25, 2021

View reviewed changes

Further fix for narrow screens.

adecce2

c-shultz requested a review from bborman22 February 25, 2021 17:38

bborman22 approved these changes Feb 25, 2021

View reviewed changes

c-shultz merged commit e65c7d9 into calypso-cleanup Feb 25, 2021

c-shultz deleted the replace-modal-component branch February 25, 2021 18:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump @wordpress version and use Modal component for shipping label purchase #2336

Bump @wordpress version and use Modal component for shipping label purchase #2336

c-shultz commented Feb 22, 2021 •

edited

Loading

c-shultz Feb 22, 2021

c-shultz Feb 22, 2021

c-shultz Feb 22, 2021

c-shultz Feb 22, 2021

c-shultz Feb 22, 2021

bborman22 left a comment

c-shultz Feb 25, 2021

c-shultz commented Feb 25, 2021

bborman22 left a comment

c-shultz commented Feb 25, 2021

bborman22 left a comment

	&.label-purchase-modal {
	height: calc(100% - 10vmin);
	width: calc(100% - 10vmin);
	max-height: 1080px;
	max-width: 1920px;
	box-shadow: rgba(151,150,150,.5) 2px 2px 7px;
	background-color: $studio-gray-0;

	@include breakpoint( '<660px' ) {
	&.dialog.card {
	height: 100%;
	max-height: 100%;
	width: 100%;
	max-width: 100%;
	}
	}

Bump @wordpress version and use Modal component for shipping label purchase #2336

Bump @wordpress version and use Modal component for shipping label purchase #2336

Conversation

c-shultz commented Feb 22, 2021 • edited Loading

Description

Related issue(s)

Steps to reproduce & screenshots/GIFs

c-shultz Feb 22, 2021

Choose a reason for hiding this comment

c-shultz Feb 22, 2021

Choose a reason for hiding this comment

c-shultz Feb 22, 2021

Choose a reason for hiding this comment

c-shultz Feb 22, 2021

Choose a reason for hiding this comment

c-shultz Feb 22, 2021

Choose a reason for hiding this comment

bborman22 left a comment

Choose a reason for hiding this comment

c-shultz Feb 25, 2021

Choose a reason for hiding this comment

c-shultz commented Feb 25, 2021

bborman22 left a comment

Choose a reason for hiding this comment

c-shultz commented Feb 25, 2021

bborman22 left a comment

Choose a reason for hiding this comment

c-shultz commented Feb 22, 2021 •

edited

Loading