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/16 12:49:17 UTC

[GitHub] [fineract-cn-mobile] PatelVatsalB21 opened a new pull request #140: Fix 272: Phone and Mobile bug fixed in Customer Contract Fragment

PatelVatsalB21 opened a new pull request #140:
URL: https://github.com/apache/fineract-cn-mobile/pull/140


   Fixes [FINCN-272](https://issues.apache.org/jira/browse/FINCN-272)
   Now Phone and Mobile fields are with numerical keyboard and also limit of 10 digits has been fixed in Mobile edt.
   
   https://user-images.githubusercontent.com/70195106/111311252-fe498180-8683-11eb-8c95-b24ef40a98fd.mp4
   
   - [x] Apply the `AndroidStyle.xml` style template to your code in Android Studio.
   
   - [x] Run the unit tests with `./gradlew check` to make sure you didn't break anything.
   
   - [x] 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] varsvat commented on a change in pull request #140: Fix 272: Phone and Mobile bug fixed in Customer Contract Fragment

Posted by GitBox <gi...@apache.org>.
varsvat commented on a change in pull request #140:
URL: https://github.com/apache/fineract-cn-mobile/pull/140#discussion_r595441114



##########
File path: app/src/main/res/layout/fragment_form_customer_contact.xml
##########
@@ -75,7 +75,8 @@
                         <EditText
                             android:hint="@string/optional_mobile"
                             android:id="@+id/et_mobile"
-                            android:inputType="text"
+                            android:inputType="number"
+                            android:maxLength="10"

Review comment:
       I thing there's a better workaround for this. We can make a map of country codes to the countries. We get the country from the user earlier while editing or creating a new customer, from there we can get the country code which can be appended to the 10 digit mobile number. This way , we can solve the problem of varied length of mobile numbers.




----------------------------------------------------------------
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 #140: Fix 272: Phone and Mobile bug fixed in Customer Contract Fragment

Posted by GitBox <gi...@apache.org>.
EGOR-IND commented on a change in pull request #140:
URL: https://github.com/apache/fineract-cn-mobile/pull/140#discussion_r595456520



##########
File path: app/src/main/res/layout/fragment_form_customer_contact.xml
##########
@@ -75,7 +75,8 @@
                         <EditText
                             android:hint="@string/optional_mobile"
                             android:id="@+id/et_mobile"
-                            android:inputType="text"
+                            android:inputType="number"
+                            android:maxLength="10"

Review comment:
       > I thing there's a better workaround for this. We can make a map of country codes to the countries. We get the country from the user earlier while editing or creating a new customer, from there we can get the country code which can be appended to the 10 digit mobile number. This way , we can solve the problem of varied length of mobile numbers.
   
   Yup that's a good solution, I think so.




----------------------------------------------------------------
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 #140: Fix 272: Phone and Mobile bug fixed in Customer Contract Fragment

Posted by GitBox <gi...@apache.org>.
EGOR-IND commented on a change in pull request #140:
URL: https://github.com/apache/fineract-cn-mobile/pull/140#discussion_r595250166



##########
File path: app/src/main/res/layout/fragment_form_customer_contact.xml
##########
@@ -75,7 +75,8 @@
                         <EditText
                             android:hint="@string/optional_mobile"
                             android:id="@+id/et_mobile"
-                            android:inputType="text"
+                            android:inputType="number"
+                            android:maxLength="10"

Review comment:
       Actually there is no correct solution because in some apps I saw a feature where the max length varies according to region. So setting a static maxlength wouldn't solve this issue exactly.




----------------------------------------------------------------
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 #140: Fix 272: Phone and Mobile bug fixed in Customer Contract Fragment

Posted by GitBox <gi...@apache.org>.
PatelVatsalB21 commented on a change in pull request #140:
URL: https://github.com/apache/fineract-cn-mobile/pull/140#discussion_r595253680



##########
File path: app/src/main/res/layout/fragment_form_customer_contact.xml
##########
@@ -75,7 +75,8 @@
                         <EditText
                             android:hint="@string/optional_mobile"
                             android:id="@+id/et_mobile"
-                            android:inputType="text"
+                            android:inputType="number"
+                            android:maxLength="10"

Review comment:
       I found max length by international telecommunication union is 15 should i make it 15?




----------------------------------------------------------------
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 #140: Fix 272: Phone and Mobile bug fixed in Customer Contract Fragment

Posted by GitBox <gi...@apache.org>.
PatelVatsalB21 commented on a change in pull request #140:
URL: https://github.com/apache/fineract-cn-mobile/pull/140#discussion_r595236378



##########
File path: app/src/main/res/layout/fragment_form_customer_contact.xml
##########
@@ -75,7 +75,8 @@
                         <EditText
                             android:hint="@string/optional_mobile"
                             android:id="@+id/et_mobile"
-                            android:inputType="text"
+                            android:inputType="number"
+                            android:maxLength="10"

Review comment:
       Oh sorry it should be 15. Shouldn't 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 pull request #140: Fix 272: Phone and Mobile bug fixed in Customer Contract Fragment

Posted by GitBox <gi...@apache.org>.
EGOR-IND commented on pull request #140:
URL: https://github.com/apache/fineract-cn-mobile/pull/140#issuecomment-802229121


   > Hey @EGOR-IND , @varsvat i changed it to 15(max digits any country can have by law). Let it be to scope of this respective pr.
   
   Okay no problem


-- 
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 #140: Fix 272: Phone and Mobile bug fixed in Customer Contract Fragment

Posted by GitBox <gi...@apache.org>.
varsvat commented on a change in pull request #140:
URL: https://github.com/apache/fineract-cn-mobile/pull/140#discussion_r595694390



##########
File path: app/src/main/res/layout/fragment_form_customer_contact.xml
##########
@@ -75,7 +75,8 @@
                         <EditText
                             android:hint="@string/optional_mobile"
                             android:id="@+id/et_mobile"
-                            android:inputType="text"
+                            android:inputType="number"
+                            android:maxLength="10"

Review comment:
       I think you misunderstood me. What I am saying is that we will add a map which would be static in the code base , so while using the country code from that map doesn't need the user to be online. I hope that makes sense.
   




----------------------------------------------------------------
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 #140: Fix 272: Phone and Mobile bug fixed in Customer Contract Fragment

Posted by GitBox <gi...@apache.org>.
varsvat commented on a change in pull request #140:
URL: https://github.com/apache/fineract-cn-mobile/pull/140#discussion_r595442042



##########
File path: app/src/main/res/layout/fragment_form_customer_contact.xml
##########
@@ -75,7 +75,8 @@
                         <EditText
                             android:hint="@string/optional_mobile"
                             android:id="@+id/et_mobile"
-                            android:inputType="text"
+                            android:inputType="number"
+                            android:maxLength="10"

Review comment:
       @EGOR-IND @PatelVatsalB21 @jawidMuhammadi please correct me if I am wrong anywhere.
   
   > I thing there's a better workaround for this. We can make a map of country codes to the countries. We get the country from the user earlier while editing or creating a new customer, from there we can get the country code which can be appended to the 10 digit mobile number. This way , we can solve the problem of varied length of mobile numbers.
   
   




----------------------------------------------------------------
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 #140: Fix 272: Phone and Mobile bug fixed in Customer Contract Fragment

Posted by GitBox <gi...@apache.org>.
EGOR-IND commented on a change in pull request #140:
URL: https://github.com/apache/fineract-cn-mobile/pull/140#discussion_r595231823



##########
File path: app/src/main/res/layout/fragment_form_customer_contact.xml
##########
@@ -75,7 +75,8 @@
                         <EditText
                             android:hint="@string/optional_mobile"
                             android:id="@+id/et_mobile"
-                            android:inputType="text"
+                            android:inputType="number"
+                            android:maxLength="10"

Review comment:
       @PatelVatsalB21 If I'm not wrong then there is no international limit of phone number length. Actually phone number length vary from region to region. So limiting the length to 10 wouldn't be good idea.




----------------------------------------------------------------
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 #140: Fix 272: Phone and Mobile bug fixed in Customer Contract Fragment

Posted by GitBox <gi...@apache.org>.
EGOR-IND commented on a change in pull request #140:
URL: https://github.com/apache/fineract-cn-mobile/pull/140#discussion_r595262296



##########
File path: app/src/main/res/layout/fragment_form_customer_contact.xml
##########
@@ -75,7 +75,8 @@
                         <EditText
                             android:hint="@string/optional_mobile"
                             android:id="@+id/et_mobile"
-                            android:inputType="text"
+                            android:inputType="number"
+                            android:maxLength="10"

Review comment:
       As I told you above making it 15 will solve the problem for the region with phone number length 15, what about regions with phone number length less than 15. @jawidMuhammadi what are views on this. I think make it 15 for now, maybe later we can add a feature for this.




----------------------------------------------------------------
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 #140: Fix 272: Phone and Mobile bug fixed in Customer Contract Fragment

Posted by GitBox <gi...@apache.org>.
PatelVatsalB21 commented on a change in pull request #140:
URL: https://github.com/apache/fineract-cn-mobile/pull/140#discussion_r595678270



##########
File path: app/src/main/res/layout/fragment_form_customer_contact.xml
##########
@@ -75,7 +75,8 @@
                         <EditText
                             android:hint="@string/optional_mobile"
                             android:id="@+id/et_mobile"
-                            android:inputType="text"
+                            android:inputType="number"
+                            android:maxLength="10"

Review comment:
       Let it be simple or else like countries list in details it won't work offline. I will make it Max 15 digits as allowed. Above 15 no country is allowed to use numbers.




----------------------------------------------------------------
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 #140: Fix 272: Phone and Mobile bug fixed in Customer Contract Fragment

Posted by GitBox <gi...@apache.org>.
PatelVatsalB21 commented on pull request #140:
URL: https://github.com/apache/fineract-cn-mobile/pull/140#issuecomment-802221426


   Hey @EGOR-IND , @varsvat i changed it to 15(max digits any country can have by law). Let it be to scope of this respective pr.


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