Skip to content
This repository has been archived by the owner on Nov 5, 2024. It is now read-only.

Refactor TransactionCard UI to support compactMode #3623

Closed
wants to merge 4 commits into from

Conversation

p42rthicle
Copy link
Contributor

@p42rthicle p42rthicle commented Oct 15, 2024

Pull request (PR) checklist

Please check if your pull request fulfills the following requirements:

  • I've read the Contribution Guidelines and my PR doesn't break the rules.
  • I've read and understand the Developer Guidelines.
  • I confirm that I've run the code locally and everything works as expected.
  • My PR includes only the necessary changes to fix the issue (i.e., no unnecessary files or lines of code are changed).
  • 🎬 I've attached a screen recording of using the new code to the next paragraph (if applicable).

Screen recording of your changes (if applicable):

transaction-card-compact.mp4

What's changed?

Describe with a few bullets what's new:

  • I've refactored the transaction card to support compact mode.
  • In the compact mode, the transaction card does not show
    • The account badges
    • Categories badges
    • Tags row
    • Description
  • Hides the due date and the Skip or Pay/Get buttons in a toggleable expand icon
  • I have refactored the components into individual reusable composables for better readability and understanding of the file
  • The TypeAmountCurrency in the non-compact card is also now TitleTypeAmountCurrencyRow just without showing the title
  • Have handled cases when the user might enter huge currency or huge amount.
  • Additionally, I have introduced a new string value for the case when the recurring transaction is paidFor and is of INCOME type
Before After
Old cards video Compact cards video
Old description Updated description

Risk factors

What may go wrong if we merge your PR?

  • The paparazzi screenshot for the :screen:transactions was breaking when the compactModeEnabled was set to true
  • I haven't changed any functionality just the UI so logic shouldn't break.
  • The new introduced string value income_received needs to be edited for the two languages (Have added Todo comments)

In what cases won't your code work?

  • Should work since its a simple Bool Feature

Does this PR close any GitHub issues? (do not delete)

Troubleshooting GitHub Actions (CI) failures ❌

Pull request checks failing? Read our CI Troubleshooting guide.

@p42rthicle
Copy link
Contributor Author

🚧 WIP
Have refactored the TransactionCard to support compactMode

@p42rthicle
Copy link
Contributor Author

Hi @ILIYANGERMANOV, how do I take care of the failing screenshot test here?
:screen:transactions:test - TransactionsPaparazziTest is failing here.

Here is the snapshot of what it expects.

Here, in the empty upcoming transaction card, there is the "deleted" tag that is shown however in my implementation of the compact mode I have omitted tags.
Attaching screen recording of current implementation for your reference.

compactmode_transaction_card_current_state.mp4

Can you help me out so I can proceed? Thanks!

@p42rthicle p42rthicle marked this pull request as ready for review October 16, 2024 11:10
@github-actions github-actions bot added the Stale No activity, will be automatically closed soon. label Oct 19, 2024
@p42rthicle
Copy link
Contributor Author

@ILIYANGERMANOV
Could you review this?

@github-actions github-actions bot removed the Stale No activity, will be automatically closed soon. label Oct 20, 2024
@ILIYANGERMANOV
Copy link
Collaborator

Hey @p42rthicle sorry I missed this PR because the CI was failing and it seemed dangerous to me

@ILIYANGERMANOV ILIYANGERMANOV added the dangerous PR that has risk of breaking something label Oct 20, 2024
@p42rthicle
Copy link
Contributor Author

Yes, I think it's some lint checks that are failing. How do I work on it?

Copy link
Collaborator

@ILIYANGERMANOV ILIYANGERMANOV left a comment

Choose a reason for hiding this comment

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

Your compact mode isn't showing accounts and categories. Accounts and categories are essential data. Instead of showing one big INCOME/EXPENSE icon let's display the other useful info. Also how does it work for transfers?

@ILIYANGERMANOV ILIYANGERMANOV added the requested changes Changes are needed. Please, apply them label Oct 20, 2024
@p42rthicle
Copy link
Contributor Author

I purposely omitted the categories and accounts info as I wanted the compact mode to display just bare minimum information. However, if you suggest I can add those in the compact mode.
What do I omit then in that case? Just the #tags and description (and the INCOME/EXPENSE ICON)? The color on the amount text can suggest if it was an income or an expense
For transfers, I am showing the transfers Icon and amount and no accounts right now. Do you suggest I add those too and make changes?

@github-actions github-actions bot added the Stale No activity, will be automatically closed soon. label Oct 25, 2024
@p42rthicle
Copy link
Contributor Author

@ILIYANGERMANOV

@github-actions github-actions bot added Stale No activity, will be automatically closed soon. and removed Stale No activity, will be automatically closed soon. labels Oct 27, 2024
@github-actions github-actions bot closed this Nov 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dangerous PR that has risk of breaking something requested changes Changes are needed. Please, apply them Stale No activity, will be automatically closed soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Compact-Sized Transaction Cards with Toggle Option
2 participants