-
Notifications
You must be signed in to change notification settings - Fork 555
fix: Improve UI of Login and SignUp Activity #554
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
Conversation
|
This is my first PR here. Can anyone guide how things goes here. |
simarsingh24
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are following eventbrite app can you confirm this UI is similar to Eventbrite's? Also the email logo and email text is too close. Please also follow commit guidelines!
app/src/main/res/values/strings.xml
Outdated
|
|
||
| <!--eventyay--> | ||
| <string name="eventyay_logo">eventyay</string> | ||
| <string name="eventyay_logo">Eventyay</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be capitalised
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I will update these changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GOVINDDIXIT Please use vector icons too
app/src/main/res/values/colors.xml
Outdated
| <color name="black">#000000</color> | ||
| <color name="grey">#ece9e9</color> | ||
| <color name="greyMore">#918e8e</color> | ||
| <color name="white">#ffffff</color> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need of adding white color. Just use @android:color/white
|
@dreadpool2 review please |
|
@nikit19 review please |
nikit19
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Just this minor change and we are good to go.
Also could you update the screenshot after making the changes
| android:text="@string/sign_up" /> | ||
| android:text="@string/sign_up" | ||
| android:textColor="@android:color/white" | ||
| android:background="@color/colorPrimary"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use backgroundTint instead
|
@nikit19 review pls |
Fixes #553
Screenshots for the change:


Before:-
After:-

