Payouts: Show bank reference key on payout details page #9886
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Significance: minor | ||
| Type: add | ||
|
|
||
| Show Bank reference key on top of the payout details page, whenever available. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
| /** | ||
| * External dependencies | ||
| */ | ||
| import React, { useState } from 'react'; | ||
| import { dateI18n } from '@wordpress/date'; | ||
| import { __, sprintf } from '@wordpress/i18n'; | ||
| import moment from 'moment'; | ||
|
|
@@ -70,7 +70,7 @@ interface SummaryItemProps { | |
| label: string; | ||
| value: string | JSX.Element; | ||
| valueClass?: string | false; | ||
| detail?: string | JSX.Element; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -102,35 +102,22 @@ const SummaryItem: React.FC< SummaryItemProps > = ( { | |
| </li> | ||
| ); | ||
|
|
||
| interface DepositDateItemProps { | ||
| deposit: CachedDeposit; | ||
| } | ||
|
|
||
| const DepositDateItem: React.FC< DepositDateItemProps > = ( { deposit } ) => { | ||
| let depositDateLabel = __( 'Payout date', 'woocommerce-payments' ); | ||
| if ( ! deposit.automatic ) { | ||
| depositDateLabel = __( 'Instant payout date', 'woocommerce-payments' ); | ||
| } | ||
| if ( deposit.type === 'withdrawal' ) { | ||
| depositDateLabel = __( 'Withdrawal date', 'woocommerce-payments' ); | ||
| } | ||
|
|
||
| const [ isCopied, setIsCopied ] = useState( false ); | ||
|
|
||
| return ( | ||
| <SummaryItem | ||
| key="depositDate" | ||
| label={ | ||
|
|
@@ -142,16 +129,85 @@ export const DepositOverview: React.FC< DepositOverviewProps > = ( { | |
| ) | ||
| } | ||
| value={ <DepositStatusIndicator deposit={ deposit } /> } | ||
| detail={ | ||
| <> | ||
| { deposit.bankAccount } | ||
| <br /> | ||
| Bank reference ID:{ ' ' } | ||
| { deposit.bank_reference_key ? ( | ||
| <> | ||
| <span className="woopayments-payout-bank-reference-id"> | ||
| { deposit.bank_reference_key } | ||
| </span> | ||
| <button | ||
| type="button" | ||
| className={ `woopayments-payout-bank-reference-id-copy-button ${ | ||
| isCopied ? 'state--copied' : '' | ||
| }` } | ||
| aria-label={ __( | ||
| 'Copy bank reference ID to clipboard', | ||
| 'woocommerce-payments' | ||
| ) } | ||
| title={ __( | ||
| 'Copy to clipboard', | ||
| 'woocommerce-payments' | ||
| ) } | ||
| onClick={ () => { | ||
| const bankReferenceId = document.querySelector( | ||
| '.woopayments-payout-bank-reference-id' | ||
| )?.textContent; | ||
|
There was a problem hiding this comment. How can we get the value without fishing around in the DOM (clue: There was a problem hiding this comment. Awesome hint! I wonder how I missed it 😄 Tweaked it here - af176bd |
||
|
|
||
| if ( bankReferenceId ) { | ||
| navigator.clipboard.writeText( | ||
| bankReferenceId | ||
| ); | ||
| } | ||
|
|
||
| if ( ! isCopied ) { | ||
| setIsCopied( true ); | ||
| setTimeout( () => { | ||
|
There was a problem hiding this comment. Need to take care with This article looks reasonable, there may be better info out there (especially on React official docs): https://felixgerschau.com/react-hooks-settimeout/ There was a problem hiding this comment. Very helpful and a great concept I learned today! I have implemented it in the CopyButton component. I hope I have done it correctly. Would appreciate if you could check once 🙏🏼 |
||
| setIsCopied( false ); | ||
| }, 2000 ); | ||
| } | ||
| } } | ||
| > | ||
| <i></i> | ||
| </button> | ||
| </> | ||
| ) : ( | ||
| 'N/A' | ||
| ) } | ||
| </> | ||
| } | ||
| /> | ||
| ); | ||
| }; | ||
| interface DepositOverviewProps { | ||
| deposit: CachedDeposit | undefined; | ||
| } | ||
|
|
||
| export const DepositOverview: React.FC< DepositOverviewProps > = ( { | ||
| deposit, | ||
| } ) => { | ||
| if ( ! deposit ) { | ||
| return ( | ||
| <InlineNotice icon status="error" isDismissible={ false }> | ||
| { __( | ||
| `The deposit you are looking for cannot be found.`, | ||
| 'woocommerce-payments' | ||
| ) } | ||
| </InlineNotice> | ||
| ); | ||
| } | ||
|
|
||
| const isWithdrawal = deposit.type === 'withdrawal'; | ||
|
|
||
| return ( | ||
| <div className="wcpay-deposit-overview"> | ||
| { deposit.automatic ? ( | ||
| <Card className="wcpay-deposit-automatic"> | ||
| <ul> | ||
| <DepositDateItem deposit={ deposit } /> | ||
| <li className="wcpay-deposit-amount"> | ||
| { formatExplicitCurrency( | ||
| deposit.amount, | ||
|
|
@@ -161,7 +217,7 @@ export const DepositOverview: React.FC< DepositOverviewProps > = ( { | |
| </ul> | ||
| </Card> | ||
| ) : ( | ||
| <SummaryList // For instant deposits only | ||
| label={ | ||
| isWithdrawal | ||
| ? __( | ||
|
|
@@ -172,7 +228,7 @@ export const DepositOverview: React.FC< DepositOverviewProps > = ( { | |
| } | ||
| > | ||
| { () => [ | ||
| <DepositDateItem key="dateItem" deposit={ deposit } />, | ||
| <SummaryItem | ||
| key="depositAmount" | ||
| label={ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,53 @@ | |
| font-size: 20px; | ||
| line-height: 28px; | ||
| } | ||
|
|
||
| .wcpay-summary__item-detail { | ||
| color: $dark-gray-500; | ||
| word-wrap: break-word; | ||
|
|
||
| .woopayments-payout-bank-reference-id { | ||
| font-family: monospace; | ||
| } | ||
|
|
||
| .woopayments-payout-bank-reference-id-copy-button { | ||
| line-height: 1.2em; | ||
| display: inline-flex; | ||
| background: transparent; | ||
| border: none; | ||
| border-radius: 0; | ||
| vertical-align: middle; | ||
| font-weight: normal; | ||
| cursor: pointer; | ||
| color: inherit; | ||
| margin-left: 2px; | ||
| align-items: center; | ||
|
|
||
| i { | ||
| display: block; | ||
| width: 1.2em; | ||
| height: 1.2em; | ||
| mask-image: url( 'assets/images/icons/copy.svg?asset' ); | ||
| mask-size: contain; | ||
| mask-repeat: no-repeat; | ||
| mask-position: center; | ||
| background-color: currentColor; | ||
|
|
||
| &:hover { | ||
| opacity: 0.7; | ||
| } | ||
|
|
||
| &:active { | ||
| transform: scale( 0.9 ); | ||
| } | ||
| } | ||
|
|
||
| &.state--copied i { | ||
| mask-image: url( 'assets/images/icons/check-green.svg?asset' ); | ||
| background-color: $studio-green-50; | ||
| } | ||
|
There was a problem hiding this comment. There's a lot of css (and some React behaviour) for this little copy-to-clipboard button. Could we package it into a little component? The button is nicely general-purpose so is a good candidate for an abstraction. |
||
| } | ||
| } | ||
| } | ||
|
|
||
| .wcpay-deposit-fee { | ||
|
|
@@ -52,13 +99,21 @@ | |
| margin: 0; | ||
| list-style-type: none; | ||
|
|
||
| @include breakpoint( '<660px' ) { | ||
| flex-direction: column; | ||
| } | ||
|
|
||
| .woocommerce-summary__item { | ||
| border-bottom: 0; | ||
| background-color: inherit; | ||
| color: inherit; | ||
| .woocommerce-summary__item-label:hover { | ||
| color: inherit; | ||
| } | ||
|
|
||
| @include breakpoint( '<660px' ) { | ||
| border-right: none; | ||
| } | ||
| } | ||
|
|
||
| .wcpay-deposit-amount { | ||
|
|
@@ -67,6 +122,12 @@ | |
| text-align: right; | ||
| font-size: 36px; | ||
| line-height: 82px; | ||
|
|
||
| @include breakpoint( '<660px' ) { | ||
| line-height: 36px; | ||
| text-align: left; | ||
| border-top: 1px solid $studio-gray-5; | ||
| } | ||
|
There was a problem hiding this comment. What are we tweaking for smaller (<660px) screens? Should we make that behaviour the default, and progressively enhance on larger screens (i.e. mobile first)? There was a problem hiding this comment. We are making the font size smaller, making it left aligned and adding a border on top. The layout by itself - overall is desktop first. The columns have a sideways scroll. My gut feel is that merchants would mostly use a tablet or a laptop to check WP admin. Hence would prefer keeping it as-is, but open to revisiting that! The padding property within the media query is redundant though, removed it - 8bda02c. There was a problem hiding this comment.
Why? Ideally the style changes for smaller screens are minimal (so it's easy to maintain), this sounds like a lot! If you are adding different behaviour for different screen sizes I'd recommend including details in the PR description – I see you have a screenshot, but even better if you explain why the changes are needed. There was a problem hiding this comment.
I have added a note on why we are doing the CSS change, in the PR description
|
||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this align with the types for the component we are overriding (
SummaryNumberfrom Woo core?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original SummaryItemNumber ( child within the summary list ) , does not have a detail prop. It was introduced in the customized
SummaryListItemfor the Deposit Overview header.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to this PR, but I think we should avoid inheriting/overriding components like this. We risk depending on implementation details, so our plugin is sensitive to changes in Woo core.
Ideally we'd either: