Replace the rest of the dialog components #2340
Conversation
c-shultz
changed the title
| <div className="carrier-accounts__settings-cancel-dialog-header"> | ||
| <h2 className="carrier-accounts__settings-cancel-dialog-title"> | ||
| {translate('Cancel connection')} | ||
| </h2> | ||
| <button | ||
| className="carrier-accounts__settings-cancel-dialog-close-button" | ||
| onClick={onCancel} | ||
| > | ||
| <Gridicon icon="cross"/> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
I like how things got simplified here! 🚀
| @@ -46,13 +45,13 @@ const ReprintDialog = props => { | |||
| ]; | |||
|
|
|||
| return ( | |||
| <Dialog | |||
| <ButtonModal | |||
| isVisible={ Boolean( reprintDialog && reprintDialog.labelId === labelId ) } | |||
| onClose={ onClose } | |||
| buttons={ buttons } | |||
There was a problem hiding this comment.
The order of the buttons is switched between the calypso-cleanup branch and this one.
On calypso-cleanup, the dialog looks as follows:
Whereas on this branch:
I noticed the buttons now have the float: right rule which they didn't have before which is likely causing this issue.
| describe( 'when buttons are passed as generic objects', () => { | ||
| const props = { | ||
| isVisible: true, | ||
| buttons: [ { label: '1', action: 'yes' }, { label: '2', action: 'no' } ], | ||
| } | ||
| const wrapper = mount( <ButtonModal { ...props } /> ); | ||
| it( '2 buttons rendered', () => { | ||
| expect( wrapper.find( 'button' ).length ).toBe( 2 ); | ||
| } ); | ||
| } ); | ||
| describe( 'when buttons are passed as Button components', () => { | ||
| const props = { | ||
| isVisible: true, | ||
| buttons: [ <Button />, <Button /> ], | ||
| } | ||
| const wrapper = mount( <ButtonModal { ...props } /> ); | ||
| it( '2 buttons rendered', () => { | ||
| expect( wrapper.find( 'button' ).length ).toBe( 2 ); | ||
| } ); |
There was a problem hiding this comment.
Maybe it would be useful to check if the keys are (re)mapped as expected when the buttons are created, or if the props are passed to child components.
| @@ -79,7 +78,7 @@ const RefundDialog = props => { | |||
| <dt>{ translate( 'Amount eligible for refund' ) }</dt> | |||
| <dd>{ getRefundableAmount() }</dd> | |||
| </dl> | |||
| </Dialog> | |||
| </ButtonModal> | |||
There was a problem hiding this comment.
The button order is different in this dialog too.
client/extensions/woocommerce/woocommerce-services/views/shipping-label/tracking-modal/index.js
Show resolved
Hide resolved
client/extensions/woocommerce/woocommerce-services/views/packages/package-dialog.js
Show resolved
Hide resolved
client/extensions/woocommerce/woocommerce-services/views/packages/predefined-packages.js
Outdated
Show resolved
Hide resolved
Use double quotes Co-authored-by: Waclaw Jacek <waclaw.jacek@automattic.com>
…ges/predefined-packages.js Use double quotes Co-authored-by: Waclaw Jacek <waclaw.jacek@automattic.com>
…ges/package-dialog.js Use double quotes Co-authored-by: Waclaw Jacek <waclaw.jacek@automattic.com>
Co-authored-by: Waclaw Jacek <waclaw.jacek@automattic.com>
…er-accounts/list-item.js Co-authored-by: Waclaw Jacek <waclaw.jacek@automattic.com>
|
There's some good issues here found by @waclawjacek. I need to rework some style conflicts which has increased the scope a good amount. I'm going to pause on this, leave the branch, and plan on returning to this once our team is focusing on shipping again. |
Description
Removes the rest of the Calypso Dialog components. To limit the amount of changes I would need to make for each use of the
Dialogcomponent, I created a wrapper for the Button from @wordpress/components calledButtonModalthat emulates theDialogcomponent. Mostly this new component is just a vanillaModalplus abuttonsprop that takes an array of buttons to be displayed at the bottom of the modal.Related issue(s)
#2333
Steps to reproduce & screenshots/GIFs
The reviewer should double check all of the dialogs. Let me know if I can help with accessing them.
Checklist