You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2021/03/22 18:54:01 UTC
[GitHub] [fineract-cn-mobile] varsvat opened a new pull request #171: checkbox replaces with Icons for proportionality field in Edit payroll Screen
varsvat opened a new pull request #171:
URL: https://github.com/apache/fineract-cn-mobile/pull/171
Fixes [FINCN-305](https://issues.apache.org/jira/browse/FINCN-305)
In the Edit payroll screen, The checkbox in the proximity of the proportional field tempts the user to modify the proportional field right by clicking on the checkbox but to actually edit the proportional field , user has to click on the edit icon beside the delete icon and then modify it. So ,I have added a image indicating green tick or red cross for either cases instead of the checkbox as on this screen :arrow_down:
After Fixing , this is how it looks : :arrow_down:
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Apply the `AndroidStyle.xml` style template to your code in Android Studio.
- [ ] Run the unit tests with `./gradlew check` to make sure you didn't break anything.
- [ ] If you have multiple commits please combine them into one commit by squashing them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract-cn-mobile] PatelVatsalB21 commented on a change in pull request #171: Fix-305: Checkbox replaces with Icons for proportionality field in Edit payroll Screen
Posted by GitBox <gi...@apache.org>.
PatelVatsalB21 commented on a change in pull request #171:
URL: https://github.com/apache/fineract-cn-mobile/pull/171#discussion_r599432772
##########
File path: app/src/main/java/org/apache/fineract/ui/adapters/PayrollAllocationAdapter.kt
##########
@@ -28,7 +29,13 @@ class PayrollAllocationAdapter @Inject constructor()
val (accountNumber, amount, proportional) = payrollAllocation[position]
holder.tvAccount.text = accountNumber
holder.tvAmount.text = amount.toString()
- holder.cbProportional.isChecked = proportional
+ if (proportional) {
+ holder.ivproportional_checked.visibility = View.VISIBLE
+ holder.ivproportional_crossed.visibility= View.GONE
+ }else {
Review comment:
@varsvat have you locally checked code with `.\gradlew check --scan`. If not I think it will fail **checkstyle** due to no space between else and `}` this brace.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract-cn-mobile] varsvat commented on a change in pull request #171: Fix-305: Checkbox replaced with Icons for proportionality field in Edit payroll Screen
Posted by GitBox <gi...@apache.org>.
varsvat commented on a change in pull request #171:
URL: https://github.com/apache/fineract-cn-mobile/pull/171#discussion_r601103766
##########
File path: app/src/main/java/org/apache/fineract/ui/adapters/PayrollAllocationAdapter.kt
##########
@@ -28,7 +29,13 @@ class PayrollAllocationAdapter @Inject constructor()
val (accountNumber, amount, proportional) = payrollAllocation[position]
holder.tvAccount.text = accountNumber
holder.tvAmount.text = amount.toString()
- holder.cbProportional.isChecked = proportional
+ if (proportional) {
+ holder.ivproportional_checked.visibility = View.VISIBLE
+ holder.ivproportional_crossed.visibility= View.GONE
+ }else {
Review comment:
See in this image , i am on the previous commit by mplodder , but then too , the build is failing .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract-cn-mobile] EGOR-IND commented on a change in pull request #171: Fix-305: Checkbox replaced with Icons for proportionality field in Edit payroll Screen
Posted by GitBox <gi...@apache.org>.
EGOR-IND commented on a change in pull request #171:
URL: https://github.com/apache/fineract-cn-mobile/pull/171#discussion_r599728192
##########
File path: app/src/main/java/org/apache/fineract/ui/adapters/PayrollAllocationAdapter.kt
##########
@@ -28,7 +29,13 @@ class PayrollAllocationAdapter @Inject constructor()
val (accountNumber, amount, proportional) = payrollAllocation[position]
holder.tvAccount.text = accountNumber
holder.tvAmount.text = amount.toString()
- holder.cbProportional.isChecked = proportional
+ if (proportional) {
+ holder.ivproportional_checked.visibility = View.VISIBLE
+ holder.ivproportional_crossed.visibility= View.GONE
+ }else {
Review comment:
Yes, it is failing checkstyle test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract-cn-mobile] varsvat commented on a change in pull request #171: Fix-305: Checkbox replaced with Icons for proportionality field in Edit payroll Screen
Posted by GitBox <gi...@apache.org>.
varsvat commented on a change in pull request #171:
URL: https://github.com/apache/fineract-cn-mobile/pull/171#discussion_r599866951
##########
File path: app/src/main/java/org/apache/fineract/ui/adapters/PayrollAllocationAdapter.kt
##########
@@ -28,7 +29,13 @@ class PayrollAllocationAdapter @Inject constructor()
val (accountNumber, amount, proportional) = payrollAllocation[position]
holder.tvAccount.text = accountNumber
holder.tvAmount.text = amount.toString()
- holder.cbProportional.isChecked = proportional
+ if (proportional) {
+ holder.ivproportional_checked.visibility = View.VISIBLE
+ holder.ivproportional_crossed.visibility= View.GONE
+ }else {
Review comment:
What is this , I dont kno about this. Why is it failing ?
##########
File path: app/src/main/java/org/apache/fineract/ui/adapters/PayrollAllocationAdapter.kt
##########
@@ -28,7 +29,13 @@ class PayrollAllocationAdapter @Inject constructor()
val (accountNumber, amount, proportional) = payrollAllocation[position]
holder.tvAccount.text = accountNumber
holder.tvAmount.text = amount.toString()
- holder.cbProportional.isChecked = proportional
+ if (proportional) {
+ holder.ivproportional_checked.visibility = View.VISIBLE
+ holder.ivproportional_crossed.visibility= View.GONE
+ }else {
Review comment:
What is this , I dont kn about this. Why is it failing ?
##########
File path: app/src/main/java/org/apache/fineract/ui/adapters/PayrollAllocationAdapter.kt
##########
@@ -28,7 +29,13 @@ class PayrollAllocationAdapter @Inject constructor()
val (accountNumber, amount, proportional) = payrollAllocation[position]
holder.tvAccount.text = accountNumber
holder.tvAmount.text = amount.toString()
- holder.cbProportional.isChecked = proportional
+ if (proportional) {
+ holder.ivproportional_checked.visibility = View.VISIBLE
+ holder.ivproportional_crossed.visibility= View.GONE
+ }else {
Review comment:
What is this , I dont knew about this. Why is it failing ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract-cn-mobile] varsvat commented on a change in pull request #171: Fix-305: Checkbox replaced with Icons for proportionality field in Edit payroll Screen
Posted by GitBox <gi...@apache.org>.
varsvat commented on a change in pull request #171:
URL: https://github.com/apache/fineract-cn-mobile/pull/171#discussion_r601101258
##########
File path: app/src/main/java/org/apache/fineract/ui/adapters/PayrollAllocationAdapter.kt
##########
@@ -28,7 +29,13 @@ class PayrollAllocationAdapter @Inject constructor()
val (accountNumber, amount, proportional) = payrollAllocation[position]
holder.tvAccount.text = accountNumber
holder.tvAmount.text = amount.toString()
- holder.cbProportional.isChecked = proportional
+ if (proportional) {
+ holder.ivproportional_checked.visibility = View.VISIBLE
+ holder.ivproportional_crossed.visibility= View.GONE
+ }else {
Review comment:
I tried with adding space between } and else , but the check is failing again. Then, I executed the command ./gradlew check --scan before this commit itself , it is failing even then. What to do now ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract-cn-mobile] varsvat commented on a change in pull request #171: Fix-305: Checkbox replaced with Icons for proportionality field in Edit payroll Screen
Posted by GitBox <gi...@apache.org>.
varsvat commented on a change in pull request #171:
URL: https://github.com/apache/fineract-cn-mobile/pull/171#discussion_r606691596
##########
File path: app/src/main/java/org/apache/fineract/ui/adapters/PayrollAllocationAdapter.kt
##########
@@ -28,7 +29,13 @@ class PayrollAllocationAdapter @Inject constructor()
val (accountNumber, amount, proportional) = payrollAllocation[position]
holder.tvAccount.text = accountNumber
holder.tvAmount.text = amount.toString()
- holder.cbProportional.isChecked = proportional
+ if (proportional) {
+ holder.ivproportional_checked.visibility = View.VISIBLE
+ holder.ivproportional_crossed.visibility= View.GONE
+ }else {
Review comment:
Required Changes have been made and now, the TRAVIS build is also successful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract-cn-mobile] varsvat commented on a change in pull request #171: Fix-305: Checkbox replaced with Icons for proportionality field in Edit payroll Screen
Posted by GitBox <gi...@apache.org>.
varsvat commented on a change in pull request #171:
URL: https://github.com/apache/fineract-cn-mobile/pull/171#discussion_r601101258
##########
File path: app/src/main/java/org/apache/fineract/ui/adapters/PayrollAllocationAdapter.kt
##########
@@ -28,7 +29,13 @@ class PayrollAllocationAdapter @Inject constructor()
val (accountNumber, amount, proportional) = payrollAllocation[position]
holder.tvAccount.text = accountNumber
holder.tvAmount.text = amount.toString()
- holder.cbProportional.isChecked = proportional
+ if (proportional) {
+ holder.ivproportional_checked.visibility = View.VISIBLE
+ holder.ivproportional_crossed.visibility= View.GONE
+ }else {
Review comment:
I tried with adding space between } and else , but the check is failing again. Then, I executed the command `./gradlew check --scan` before this commit itself , it is failing even then. What to do now ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract-cn-mobile] PatelVatsalB21 commented on a change in pull request #171: Fix-305: Checkbox replaced with Icons for proportionality field in Edit payroll Screen
Posted by GitBox <gi...@apache.org>.
PatelVatsalB21 commented on a change in pull request #171:
URL: https://github.com/apache/fineract-cn-mobile/pull/171#discussion_r601428925
##########
File path: app/src/main/java/org/apache/fineract/ui/adapters/PayrollAllocationAdapter.kt
##########
@@ -28,7 +29,13 @@ class PayrollAllocationAdapter @Inject constructor()
val (accountNumber, amount, proportional) = payrollAllocation[position]
holder.tvAccount.text = accountNumber
holder.tvAmount.text = amount.toString()
- holder.cbProportional.isChecked = proportional
+ if (proportional) {
+ holder.ivproportional_checked.visibility = View.VISIBLE
+ holder.ivproportional_crossed.visibility= View.GONE
+ }else {
Review comment:
Try to go through failed test of class you made changes on links that would be available on scrolling up. Read carefully, there would be line number where test is failing. These reports would be less readable but you would figure it out on giving some time to it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract-cn-mobile] EGOR-IND commented on a change in pull request #171: Fix-305: Checkbox replaced with Icons for proportionality field in Edit payroll Screen
Posted by GitBox <gi...@apache.org>.
EGOR-IND commented on a change in pull request #171:
URL: https://github.com/apache/fineract-cn-mobile/pull/171#discussion_r599868773
##########
File path: app/src/main/java/org/apache/fineract/ui/adapters/PayrollAllocationAdapter.kt
##########
@@ -28,7 +29,13 @@ class PayrollAllocationAdapter @Inject constructor()
val (accountNumber, amount, proportional) = payrollAllocation[position]
holder.tvAccount.text = accountNumber
holder.tvAmount.text = amount.toString()
- holder.cbProportional.isChecked = proportional
+ if (proportional) {
+ holder.ivproportional_checked.visibility = View.VISIBLE
+ holder.ivproportional_crossed.visibility= View.GONE
+ }else {
Review comment:
Run the command that vatsal mentioned in his comment and it will show, btw I can see the issue from the highlighted code you have to put space between "}else"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract-cn-mobile] PatelVatsalB21 commented on pull request #171: Fix-305: Checkbox replaces with Icons for proportionality field in Edit payroll Screen
Posted by GitBox <gi...@apache.org>.
PatelVatsalB21 commented on pull request #171:
URL: https://github.com/apache/fineract-cn-mobile/pull/171#issuecomment-804780717
Rest looks good to me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org