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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #747 : Added Checks for empty Edittexts and for validity of Url in Update Endpoints Setting #765

Merged
merged 1 commit into from
Jun 4, 2018

Conversation

miPlodder
Copy link
Collaborator

Fixes #747

pr_747

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.

@miPlodder
Copy link
Collaborator Author

@therajanmaurya @dilpreet96 @satyan @droidchef Please review this PR.

@@ -383,6 +383,8 @@
<string name="language">Language</string>
<string name="choose_language">Choose your language</string>
<string name="notification">Notifications</string>
<string name="base_url">Base Url</string>
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be Base URL

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@luckyman20 I have used "Base Url" because the Title also shows "Base Url". It's better to have consistency in messages.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@miPlodder I think you can change that too also because URL is a short form for Uniform Resource Locater & short forms should be written in caps only.

@luckyman20
Copy link
Collaborator

luckyman20 commented Mar 20, 2018

@miPlodder Another approach you can use that check all the things when OK button is clicked, it will shorten the code.

You should also check whether it is a valid URL or not

@miPlodder
Copy link
Collaborator Author

miPlodder commented Mar 22, 2018

@luckyman20 I have made a check for validity of the URL. Moreover, when OK button is pressed, then the AlertDialog closes, we cannot prevent this behaviour, please refer this abstract class PreferenceDialogFragmentCompat, where it creates iAlert dialog and we are using it.

Moreover, it's good approach to give continuous feedback to user in forms where lesser number of fields are available. That's why I have used TextWatcher.

@therajanmaurya
Copy link
Member

Fix conflicts.

@miPlodder
Copy link
Collaborator Author

@therajanmaurya Please review this PR again.

@therajanmaurya therajanmaurya merged commit 51a6f1c into openMF:development Jun 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants