Fixes #392 : Re-designs the login page #412
Conversation
|
I feel like the Google auth button is too far down in the screen; maybe we should put it above the username/password form? |
|
@timabbott By mistake , i added the video link of old login flow. I have updated it now. Is the Google auth button still looks far down? |
|
Maybe we should move the Google login button above the hrule? I'm not sure having horizontal lines between auth methods makes sense, but a horizontal line between auth and "setup a server" does. I could also see moving "register" below that line, so the top area is "login" and below it is "other options". |
|
@timabbott Cool. Will do the changes. :) |
|
Nice, just noticed the last commit... 0fcd510 has already been merged. |
|
@Sam1301 weird, I updated to master branch but still it was showing in the lint issue, so i fixed that. |
|
maybe try again.. I can see your branch not up-to-date with master https://github.com/saketkumar95/zulip-android/commits/patch-21-login |
|
@Sam1301 okay, trying again. Thanks for the info. :) |
saketkumar
changed the title
saketkumar
changed the title
|
Can you post some screenshots for the changes in the large screens? |
| android:orientation="vertical" | ||
| android:padding="16dp" | ||
| android:visibility="gone"> | ||
|
|
| android:textAppearance="@style/Base.TextAppearance.AppCompat.Medium" | ||
| android:textColor="#fff" | ||
| android:textSize="25sp" /> | ||
|
|
There was a problem hiding this comment.
@saketkumar95 Nice work! 👍
Could you also post screenshots for different screen densities?
| private void forgotPassword() { | ||
|
|
||
| Uri uri; | ||
| if (serverIn == null || serverIn.getText().toString().isEmpty() || serverIn.getText().toString().equals("")) { |
There was a problem hiding this comment.
TextUtils.isEmpty() can be used
| @@ -16,7 +18,7 @@ | |||
| public static void transition(Context activityContext) { | |||
| // avoid invalid class cast exception | |||
| if (activityContext instanceof Activity) { | |||
| ((Activity) activityContext).overridePendingTransition(android.R.anim.fade_in, android.R.anim.fade_out); | |||
| ((Activity) activityContext).overridePendingTransition(R.anim.slide_in_right, android.R.anim.fade_out); | |||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <selector xmlns:android="http://schemas.android.com/apk/res/android" > | ||
| <item android:state_pressed="true" > | ||
| <shape android:shape="rectangle" > |
There was a problem hiding this comment.
if its not much work, it wold be awesome to have a ripple effect on long press
| android:gravity="center_horizontal" | ||
| android:orientation="vertical" | ||
| android:weightSum="1"> | ||
|
|
There was a problem hiding this comment.
extra newlines; please also check other xml files for the same
| @@ -14,4 +14,26 @@ | |||
| <style name="links.gone" parent="links"> | |||
| <item name="android:visibility">gone</item> | |||
| </style> | |||
|
|
|||
| <style name="linksLarge"> | |||
| <item name="android:layout_width">wrap_content</item> | |||
There was a problem hiding this comment.
we could abstract out common properties into a parent style and inherit from it instead
|
@saketkumar95 also can you resolve the merge conflicts? |
|
@saketkumar95 this pr is really good and almost ready to merged. If the above comments could be addressed, this can be merged soon 👍. |
|
@Sam1301 Will do this today for sure. :) |
|
@saketkumar95, your pull request has developed a merge conflict! Please review the most recent commit (f791888) for conflicting changes and resolve your pull request's merge conflicts. |
Summary of changes
This PR fixes https://github.com/zulip/zulip-android/issues/392 :
https://drive.google.com/file/d/0Bx-iZdT4RNS9NnFTd1BzYlZLVFk/view
Please make sure these boxes are checked before submitting your pull request - thanks! Guide
Code is formatted.
Run the lint tests with
./gradlew lintDebugand make sure BUILD is SUCCESSFULIf new feature is implemeted then it is compatible with Night theme too.
Commit messages are well-written.