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

feat:added login screen and passcode #4

Merged
merged 1 commit into from
Jun 4, 2018

Conversation

ivary43
Copy link
Collaborator

@ivary43 ivary43 commented Jun 2, 2018

Fixes #3

Please Add Screenshots If there are any UI changes.

Please make sure these boxes are checked before submitting your pull request - thanks!

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

@ivary43
Copy link
Collaborator Author

ivary43 commented Jun 3, 2018

@therajanmaurya please review.

app/build.gradle Outdated
@@ -98,6 +100,10 @@ dependencies {
compileOnly "org.glassfish:javax.annotation:10.0-b28" //Required by Dagger2
kapt daggerCompiler

//butterknife
implementation "com.jakewharton:butterknife:$rootProject.butterknifeVersion"
kapt "com.jakewharton:butterknife-compiler:$rootProject.butterknifeVersion"
Copy link
Member

Choose a reason for hiding this comment

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

We don't nee butterknife anymore, we are working with kotlin extension.

class LoginActivity : MifosBaseActivity(), LoginContract.View {


@BindView(R.id.et_username)
Copy link
Member

Choose a reason for hiding this comment

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

We are using kotlin extenstion, you can access ui instance using their ids

@OnClick(R.id.btn_login)
internal fun login() {

val username = et_Username.text.toString().trim()
Copy link
Member

Choose a reason for hiding this comment

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

Use camel case for naming convention like etUserName


override fun showProgress() {
showProgressbar()
Toaster.show(findViewById(android.R.id.content), getString(R.string.logging_in), Toaster.SHORT)
Copy link
Member

Choose a reason for hiding this comment

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

why toast is here.

}

override fun startLoginActivity() {
val i = Intent(this, LoginActivity::class.java)
Copy link
Member

Choose a reason for hiding this comment

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

change i to loginIntent

getMvpView.showUserLoginSuccessfully()
} else {
getMvpView.hideProgress()
getMvpView.showError("Error Logging in")
Copy link
Member

Choose a reason for hiding this comment

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

don't use hardcoded string, move this string in strings.xml

internal fun login() {

val etUsername = et_username.text.toString().trim()
val etPassword = et_password.text.toString().trim()
Copy link
Member

Choose a reason for hiding this comment

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

@ivary43 Please use camel case naming convention in xml, so change the Edit text id in xml like etUserName and etPassword

android:drawablePadding="@dimen/layout_padding_8dp"
android:drawableStart="@drawable/ic_person_black_24dp"
android:hint="@string/username"
android:id="@+id/et_username"
Copy link
Member

Choose a reason for hiding this comment

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

Make edit text it etUserName

android:drawablePadding="@dimen/layout_padding_8dp"
android:drawableStart="@drawable/ic_password_black_24dp"
android:hint="@string/password"
android:id="@+id/et_password"
Copy link
Member

Choose a reason for hiding this comment

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

make edit text id etPassword

</LinearLayout>

<Button
android:id="@+id/btn_login"
Copy link
Member

Choose a reason for hiding this comment

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

make Button id btnLogin

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.

None yet

2 participants