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.

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

Fix: Columns are misaligned on Payments->Deposits #1838

Merged
merged 4 commits into from
May 19, 2021

Conversation

naman03malhotra
Copy link
Contributor

@naman03malhotra naman03malhotra commented May 17, 2021

Fixes #1719 (comment)

I had to reopen #1719 as GlobalStep observed misalignment on the deposits page as per the above comment.

Changes proposed in this Pull Request

  • Override alignment for table header on the deposits list page.
  • Slight adjustment in the transactiond table header margin.

After fix:
image
image

Testing instructions

  • Checkout to fix/1719-desposit-col-alignment
  • Visit deposits pages and transactions page
  • Columns should be aligned now.

  • Added changelog entry (or does not apply)
  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

@naman03malhotra naman03malhotra changed the title Fix: #1719 Columns are misaligned on Payments->Deposits Fix: Columns are misaligned on Payments->Deposits May 17, 2021
@naman03malhotra naman03malhotra force-pushed the fix/1719-desposit-col-alignment branch from eea7bc6 to c3360d5 Compare May 17, 2021 20:49
@naman03malhotra naman03malhotra requested a review from a team May 17, 2021 20:50
@naman03malhotra naman03malhotra self-assigned this May 17, 2021
@kalessil kalessil self-requested a review May 18, 2021 11:18
@kalessil
Copy link
Contributor

kalessil commented May 18, 2021

Hey @naman03malhotra, looks much better now!

Found a case on transactions listing related to currency conversion marker rendering (div with class converted-amount is getting added before the amount in the case):
Screenshot from 2021-05-18 13-33-12

Would you please take a look into it?

Copy link
Contributor

@kalessil kalessil left a comment

Choose a reason for hiding this comment

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

See the screenshot.

@naman03malhotra naman03malhotra force-pushed the fix/1719-desposit-col-alignment branch from ad1c17a to 5df0e93 Compare May 19, 2021 10:18
@naman03malhotra
Copy link
Contributor Author

naman03malhotra commented May 19, 2021

Found a case on transactions listing related to currency conversion marker rendering (div with class converted-amount is getting added before the amount in the case)

Thanks, @kalessil for this, fixed it in 8c6f2cc. Nice catch 🙂
image

Copy link
Contributor

@kalessil kalessil left a comment

Choose a reason for hiding this comment

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

LGTM, :shipit: !

@kalessil
Copy link
Contributor

@naman03malhotra: this PR is approved, great job here! Would you please rebase it, to ensure the failed tests are getting green?

- override alignment for deposits list page
- slight adjustment in transaction table header margin
- Aligned data with currency conversion marker
@naman03malhotra naman03malhotra force-pushed the fix/1719-desposit-col-alignment branch from 8c6f2cc to bbe9561 Compare May 19, 2021 13:16
@naman03malhotra
Copy link
Contributor Author

Would you please rebase it, to ensure the failed tests are getting green?

@kalessil looks like this \/ workflow is failing for other PRs as well, the fix is not yet in the develop branch.
https://github.com/Automattic/woocommerce-payments/pull/1838/checks?check_run_id=2620519571

@kalessil
Copy link
Contributor

@naman03malhotra , oh I see now. Go ahead then!

@naman03malhotra naman03malhotra merged commit 4fde9a7 into develop May 19, 2021
@naman03malhotra naman03malhotra deleted the fix/1719-desposit-col-alignment branch May 19, 2021 15:41
ismaeldcom pushed a commit that referenced this pull request May 27, 2021
* Fix: #1719 Misaligned columns on Payments->Deposits
- override alignment for deposits list page.
- slight adjustment in transaction table header margin.
- changelog update.
- updated failing snapshots.
- Review changes.
- Aligned data with currency conversion marker.
ismaeldcom pushed a commit that referenced this pull request May 27, 2021
* Fix: #1719 Misaligned columns on Payments->Deposits
- override alignment for deposits list page.
- slight adjustment in transaction table header margin.
- changelog update.
- updated failing snapshots.
- Review changes.
- Aligned data with currency conversion marker.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GlobalStep] Columns are misaligned on Payments->Transactions/Disputes page
2 participants