-
Notifications
You must be signed in to change notification settings - Fork 520
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 #1914: Added text based styles #1915
Conversation
@anandwana001 @MohamedMedhat1998 @Sarthak2601 Please have a look at the description and code. This is important as you will also be reviewing PRs related to UI changes and these things will get used more often now. |
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.
LGTM, Thanks @rt4914
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.
LGTM. Thanks @rt4914 :)
@aggarwalpulkit596 Can you please resolve comments if both queries are answered? |
Merging this now but @MohamedMedhat1998 feel free to have a look and leave 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.
I have left a comment below, PTAL.
I also have one question, why do we have to use both android:fontFamily
and fontFamily
?
<style name="Caption" parent="TextView.Common1"> | ||
<item name="android:fontFamily">sans-serif-medium</item> | ||
<item name="android:textSize">14sp</item> | ||
<item name="fontFamily">sans-serif-medium</item> | ||
</style> |
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.
According to #1914 description, this should be sans-serif
not sans-serif-medium
Explanation
Fix #1914
The current UI contains a lot of common views and therefore we should define those common styles in
styles.xml
file.The styles should be defined for following items as a start and can be added more as we start updating the UI.
Note: I have not added style for
BUTTON
because we already haveStateButtonActive
.Also, these are just the starter styles, as we move to the UI part we should focus on creating common styles which can be used at multiple places.
Checklist