Skip to content
This repository was archived by the owner on Apr 11, 2026. It is now read-only.

[FEATURE] Dense account design#3428

Merged
ILIYANGERMANOV merged 11 commits intoIvy-Apps:mainfrom
shamim-emon:fix-issue-3297
Aug 26, 2024
Merged

[FEATURE] Dense account design#3428
ILIYANGERMANOV merged 11 commits intoIvy-Apps:mainfrom
shamim-emon:fix-issue-3297

Conversation

@shamim-emon
Copy link
Copy Markdown
Contributor

@shamim-emon shamim-emon commented Aug 25, 2024 •
edited
Loading

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):

What's changed?

Describe with a few bullets what's new:

  • I've updated account design of Accounts tab.

Risk factors

What may go wrong if we merge your PR?

  • No known issue.

In what cases won't your code work?

  • No known case

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

Troubleshooting GitHub Actions (CI) failures ❌

Pull request checks failing? Read our CI Troubleshooting guide.

@shamim-emon shamim-emon changed the title Dense account design [FEATURE] Dense account design Aug 25, 2024
@shamim-emon shamim-emon marked this pull request as draft August 25, 2024 10:36
@shamim-emon shamim-emon marked this pull request as ready for review August 25, 2024 10:51
ILIYANGERMANOV
ILIYANGERMANOV suggested changes Aug 25, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@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.

You must not change the default design. Instead create a new one and show if a "Compact accounts" feature flag from IvyFeatures is enabled

@ILIYANGERMANOV ILIYANGERMANOV added the requested changes Changes are needed. Please, apply them label Aug 25, 2024
ILIYANGERMANOV
ILIYANGERMANOV suggested changes Aug 25, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@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.

Nice! Starts to look good 💯 Left some comments

Comment thread shared/domain/src/main/java/com/ivy/domain/features/IvyFeatures.kt Outdated
Comment thread shared/domain/src/main/java/com/ivy/domain/features/IvyFeatures.kt Outdated
Comment thread shared/domain/src/main/java/com/ivy/domain/features/IvyFeatures.kt Outdated
Comment thread screen/accounts/src/test/java/com/ivy/accounts/AccountsTabPaparazziTest.kt
Comment thread screen/accounts/src/main/java/com/ivy/accounts/AccountsViewModel.kt Outdated
Comment thread screen/accounts/src/main/java/com/ivy/accounts/AccountsViewModel.kt Outdated
Comment thread screen/accounts/src/main/java/com/ivy/accounts/AccountsViewModel.kt Outdated
Comment thread screen/accounts/src/main/java/com/ivy/accounts/AccountsTab.kt Outdated
Comment thread screen/accounts/src/main/java/com/ivy/accounts/AccountsState.kt Outdated
Comment thread shared/domain/src/main/java/com/ivy/domain/features/IvyFeatures.kt Outdated
ILIYANGERMANOV
ILIYANGERMANOV approved these changes Aug 25, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@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.

Fix the CI and we can merge

@ILIYANGERMANOV ILIYANGERMANOV added ready to merge 🚀 PR can be merged after the CI pass and removed requested changes Changes are needed. Please, apply them labels Aug 25, 2024
@shamim-emon
Copy link
Copy Markdown
Contributor Author

shamim-emon commented Aug 26, 2024

Fix the CI and we can merge

Ok will fix it soon!

@ILIYANGERMANOV ILIYANGERMANOV merged commit a116fff into Ivy-Apps:main Aug 26, 2024
@shamim-emon shamim-emon deleted the fix-issue-3297 branch August 26, 2024 09:04
@Jack-Works
Copy link
Copy Markdown
Contributor

Jack-Works commented Aug 26, 2024

wow, I tried it, it works well, thanks!

@shamim-emon
Copy link
Copy Markdown
Contributor Author

shamim-emon commented Aug 26, 2024

wow, I tried it, it works well, thanks!

Welcome

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@ILIYANGERMANOV ILIYANGERMANOV ILIYANGERMANOV approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ready to merge 🚀 PR can be merged after the CI pass

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[FEATURE] Dense account design

3 participants

@shamim-emon @Jack-Works @ILIYANGERMANOV