Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reporting: Fix the View report link URL on Total payment volume tile, within Payment activity widget #8726

Merged
merged 17 commits into from
May 2, 2024
Merged

Reporting: Fix the View report link URL on Total payment volume tile, within Payment activity widget #8726

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9529546
Add search params for TPV to URL
Apr 29, 2024
477a606
Add changelog
Apr 29, 2024
039068e
Merge branch 'develop' into fix/8706-tpv-view-report-link
Apr 29, 2024
95c9e0c
Update test snapshot
Apr 29, 2024
eb1e2dc
Merge branch 'develop' into fix/8706-tpv-view-report-link
Apr 29, 2024
7930735
Fix typo in changelog comment
Apr 29, 2024
f81fe22
Merge branch 'develop' into fix/8706-tpv-view-report-link
jessy-p Apr 29, 2024
5008837
Merge branch 'develop' into fix/8706-tpv-view-report-link
jessy-p Apr 30, 2024
75a17d2
Show date filter explicitly
Apr 30, 2024
2c2d6b1
Update test snapshot
Apr 30, 2024
61cf53a
Clean up unwanted import
Apr 30, 2024
62bdd19
Merge branch 'develop' into fix/8706-tpv-view-report-link
May 1, 2024
75a65a4
Adjust search component height
May 1, 2024
16d12d2
code refactor to help adding links of other tiles
May 1, 2024
4b53bac
Refactor - move search function outside component.
May 2, 2024
5170cf9
Merge branch 'develop' into fix/8706-tpv-view-report-link
May 2, 2024
8cd9331
Update test snapshot
May 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/fix-8706-tpv-view-report-link
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: fix
Comment: Comment: Fix the `View report` link on Total payment volume tile on the Payment activity widget. Changes behind feature flag, and part of larger change that adds the Payment activity widget.


21 changes: 20 additions & 1 deletion client/components/payment-activity/payment-activity-data.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,18 @@ const PaymentActivityData: React.FC = () => {
const refunds = paymentActivityData?.refunds ?? 0;
const currency = paymentActivityData?.currency;

const searchTermsForTPV = [
'charge',
'payment',
'payment_failure_refund',
'payment_refund',
'refund',
'refund_failure',
'dispute',
'dispute_reversal',
'card_reader_fee',
];

return (
<div className="wcpay-payment-activity-data">
<PaymentDataTile
Expand Down Expand Up @@ -86,13 +98,20 @@ const PaymentActivityData: React.FC = () => {
reportLink={ getAdminUrl( {
page: 'wc-admin',
path: '/payments/transactions',
filter: 'advanced',
'date_between[0]': moment(
getDateRange().date_start
).format( 'YYYY-MM-DD' ),
'date_between[1]': moment( getDateRange().date_end ).format(
'YYYY-MM-DD'
),
filter: 'advanced',
...searchTermsForTPV.reduce(
( acc, term, index ) => ( {
...acc,
[ `search[${ index }]` ]: term,
} ),
{}
),
} ) }
isLoading={ isLoading }
/>
Expand Down
2 changes: 1 addition & 1 deletion client/components/payment-activity/test/__snapshots__/index.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ exports[`PaymentActivity component should render 1`] = `
</p>
<a
data-link-type="wc-admin"
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions&date_between%5B0%5D=2024-04-01&date_between%5B1%5D=2024-04-08&filter=advanced"
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions&filter=advanced&date_between%5B0%5D=2024-04-01&date_between%5B1%5D=2024-04-08&search%5B0%5D=charge&search%5B1%5D=payment&search%5B2%5D=payment_failure_refund&search%5B3%5D=payment_refund&search%5B4%5D=refund&search%5B5%5D=refund_failure&search%5B6%5D=dispute&search%5B7%5D=dispute_reversal&search%5B8%5D=card_reader_fee"
>
View report
</a>
Expand Down
2 changes: 2 additions & 0 deletions client/transactions/list/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ $gap-small: 12px;

.woocommerce-select-control__control {
height: 38px;
overflow: scroll;
align-items: flex-start;
Copy link
Member

@Jinksi Jinksi May 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Although it is a minor fix, this change isn't behind the feature flag.

Perhaps it would help communicate this change if we had 2 changelog entries:

  1. Fix for view report behind feature flag.
  2. Fix for Transaction List styles

Alternatively, relevant to this comment:

@jessy-p I was wondering if I should create a separate case for this. Would this be a blocker for the current PR?

We could move this change to a separate issue/PR. This may help us step back and look at the problem/solution separate to the Payment Activity Card.

Copy link
Member

Choose a reason for hiding this comment

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

As an alternative UX to overflow: scroll, I would consider letting the search box to increase in height when necessary, example:

image

image

(avoiding the !important if possible 😉)

Copy link
Contributor Author

@nagpai nagpai May 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Although it is a minor fix, this change isn't behind the feature flag.

I had completely missed this! Thanks for sharing!.

I would favor creating a separate issue + PR. It would be tiny but keep things clear.

I did a deeper check and it looks like we need some more work to do for responsive view ( for narrow screens ) . e.g. The Date filter with Between appears botched in mobile screen, and the search prompt disappears too in the narrow screen. I will create a separate issue for this.

Within this PR however, I will include the change @Jinksi suggested to increase the height. - 75a65a4. Since it will help immediately with how the tags appear when there are too many.

Copy link

Choose a reason for hiding this comment

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

@nagpai Thanks for rising it and @Jinksi thanks for coming up with the fix. I feel like the increase in height is the best we can do now. For the narrower (mobile) screens - I don't know if this is possible – but I'd:

  1. put the applied filter (tags) in one line (could they be horizontally scrollable?)
  2. have the x cross sign for the user to clear the filters visible
  3. have the next (2nd) line empty so that the user can search if they want

In a nutshell, I'd prioritise the user's ability to search over seeing all the applied filters.

Ugh, does it make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rogermattic Thanks, Magda! I will add this guidance in the new issue I create for improving the mobile UI 🙏 !

Copy link

Choose a reason for hiding this comment

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

Thanks @nagpai ! if you have anything for me to look at, just ping me:)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have created #8781 to track this change here!

}
}

Expand Down
Loading