Using styles xml #112
Using styles xml #112
Conversation
|
Thanks @perlajarillo! I'll find someone on the android side to review this shortly |
|
No problem @nickoneill! Please let me know if some changes are required. |
|
Hi, I can take a look at this soon, hopefully this evening. Sorry for the delay -- I was unavailable this weekend! |
There was a problem hiding this comment.
This is super awesome! I only found one place where the UI appears different and I looked at many (perhaps not all) of the views.
I do have some comments around what belongs in styles.xml vs what doesn't.
I'm also wondering whether we should do something about the lint errors in styles.xml that many attributes require APIs higher than the minimum API. We could suppress these or just not worry about it at the moment. What do you think?
| android:layout_width="match_parent" | ||
| android:layout_height="match_parent" | ||
| android:background="@color/background_color" | ||
| > | ||
| android:background="@color/background_color"> |
There was a problem hiding this comment.
Normally I like to wrap the closing > or /> in the XML to the next line, to reduce diffs when adding new lines to the XML.
You don't have to do this but I'm just mentioning it as an optional comment!
|
Hi @dektar , thank you for the feedback, I will work in the improvements. |
|
Hi @dektar . I have completed the changes you suggested. In my personal opinion you don't need to worry about the attributes that require APIs higher than the minimum API for now. I have reviewed once more all the views but please let me know if I forgot something. |
| <TextView | ||
| style="@style/aboutParagraphTextStyle" | ||
| android:text="@string/rate_us_prompt" /> | ||
| <!-- wasn't in next btn <item name="android:layout_margin">@dimen/layout_margin_button</item>--> |
There was a problem hiding this comment.
Can you resolve or remove this comment?
There was a problem hiding this comment.
I forgot to remove it, but now it is gone.
Removing unnecessary comment.
|
Thanks. Do you want to try to resolve the conflicts with the base branch? |
|
@dektar I resolved the conflicts. I thinks this PR is ready to go, but let me know if something else is required. |
Related to #20: Make more use of styles.xml rather than formatting every view.