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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Income and expenses in alphabetic order #3192

Closed
wants to merge 2 commits into from

Conversation

luispacheco-dev
Copy link
Contributor

Pull Request (PR) Checklist

Please check if your pull request fulfills the following requirements:

  • The PR is submitted to the main branch.
  • 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.
  • 馃幀 I've attached a screen recoding of the changes.

Screenshot_20240511_170600

@@ -598,7 +598,7 @@ class EditTransactionViewModel @Inject constructor(
private fun createCategory(data: CreateCategoryData) {
viewModelScope.launch {
categoryCreator.createCategory(data) {
categories.value = categoryRepository.findAll().toImmutableList()
categories.value = categoryRepository.findAll().sortedBy { it.name.value }.toImmutableList()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This shouldn't be the default behavior. Most users prefer to see the categories in the order that they've put them in the Categories screen. Make this feature a Setting using Features and IvyFeatures and by default it must be off

@@ -632,7 +632,7 @@ class EditTransactionViewModel @Inject constructor(
private fun editCategory(updatedCategory: Category) {
viewModelScope.launch {
categoryCreator.editCategory(updatedCategory) {
categories.value = categoryRepository.findAll().toImmutableList()
categories.value = categoryRepository.findAll().sortedBy { it.name.value }.toImmutableList()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This code is duplicated 3 times, extract it as a function

@ILIYANGERMANOV ILIYANGERMANOV added the requested changes Changes are needed. Please, apply them label May 12, 2024
@luispacheco-dev
Copy link
Contributor Author

I'm going to close this PR because I sent other with the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
requested changes Changes are needed. Please, apply them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants