Massively simplify styling #696

SUPERCILEX · 2017-05-09T05:23:44Z

@samtstern I was going through the sign-in process for my app because I updated some Facebook/Twitter settings and it looked pretty ugly: 😱

Long story short, I forgot to style FirebaseUI correctly. Oops! 😄 Anyway, the point of this PR is to make styling easier, or more accurately, unnecessary. At the moment, styling requires a bunch of manual work so this PR basically styles FirebaseUI with the app's default look n' feel for free. Yay! 🚀 🎉 😄

Here's a really large and gnarly table with comparison screenshots of the default no config theme before and after this PR:

Old	New

@samtstern Lol, it took me so long to make that table. 😆 This PR is a WIP because I still haven't looked through my code to clean stuff up and the existing themes look pretty dang ugly right now (their colorAccent is yucky which makes all the buttons pretty bad).

@samtstern What are your thoughts on this before I move forward and update our sample app themes? I was almost thinking of removing theme customization completely and just using the app's default theme, but I feel like that might be a little extreme... thoughts, comments, concerns, random outbursts? 😆

SUPERCILEX · 2017-05-09T05:26:04Z

auth/src/main/res/values/colors.xml

-    <color name="authui_inputTextColorDark">#000000</color>
+    <color name="colorPrimary">#3F51B5</color>
+    <color name="colorPrimaryDark">#303F9F</color>
+    <color name="colorAccent">#FF4081</color>


This is where all the magic happens: the app's theme overrides this so we get theme customization for free. However, we need to keep these values to be able to build the auth module.

I do think your new colors look better, however I think the original color schemes were set by the person who made the UX mocks so I am hesitant to change them based on our personal preference.

I'll see if someone can weigh in on that. But warning I may have to ask you to revert the colors, which will make your beautiful table obsolete :-/

@samtstern Wait a sec, I think you're confused by the purpose of this PR and the pure awesomeness going on behind the scenes! 😄 Those colors don't mean anything are the default Android Studio setup colors:

Hex value Color

<color name="colorPrimary">#3F51B5</color>

<color name="colorPrimaryDark">#303F9F</color>

<color name="colorAccent">#FF4081</color>

(I'm getting good at those tables 😆)

The colors you see from the screenshots in the original post are from the dev's app theme (in this case FirebaseUI): https://github.com/firebase/FirebaseUI-Android/blob/master/app/src/main/res/values/colors.xml#L2-L4

So the idea is that with no configuration at all, a dev can just add FirebaseUI and it will automatically style itslef with their app's theme because their colorPrimary, colorPrimaryDark, and colorAccent are overriding FirebaseUI's. Thus the colors in the FirebaseUI style are meaningless and are just there so that we can compile the auth module.

I see. I wasn't thinking it through all the way. Before this change the colors in our theme mattered since we were not leaning on AppCompat, so if the developer did not pass a theme those colors would show up.

Now because we are leaning on AppCompat our theme will almost never show up (unless the dev is not using AppCompat, right?)

@samtstern Yup and yup! I'm happy to change the colors back to what they were since they shouldn't matter; what I really want is a way to error if the dev doesn't override our colors, but I don't think that's possible.

samtstern

@SUPERCILEX some minor comments. Feel free to go crazy with the sample themes, but let's hold off on the other change (removing themes entirely and only relying on the app's theme).

samtstern · 2017-05-09T16:33:06Z

auth/src/main/res/values/styles.xml

@@ -2,17 +2,10 @@
 <resources>

    <style name="FirebaseUI" parent="Theme.AppCompat.Light.DarkActionBar">
-        <item name="colorPrimary">@color/authui_colorPrimary</item>


So are all of the deleted values OK because they default to relying on colorPrimary, colorPrimaryDark, and colorAccent?

Yup! To build on what I said above, this PR makes heavy usage of AppCompat which takes care of almost all of the styling for us using the dev's overriden colors.

samtstern · 2017-05-09T16:36:26Z

auth/src/main/res/values/colors.xml

-    <color name="authui_inputTextColorDark">#000000</color>
+    <color name="colorPrimary">#3F51B5</color>
+    <color name="colorPrimaryDark">#303F9F</color>
+    <color name="colorAccent">#FF4081</color>


I do think your new colors look better, however I think the original color schemes were set by the person who made the UX mocks so I am hesitant to change them based on our personal preference.

I'll see if someone can weigh in on that. But warning I may have to ask you to revert the colors, which will make your beautiful table obsolete :-/

samtstern · 2017-05-09T16:37:02Z

auth/src/main/res/values/styles.xml

        <item name="android:textSize">16sp</item>
-        <item name="android:lineSpacingExtra">8sp</item>


Are these all the same as defaults too?

Not really, they're using @style/Widget.AppComat.TextInputLayout which takes care of the styling for us. (And of course AppCompat is following the material spec.)

samtstern · 2017-05-09T16:37:24Z

auth/src/main/res/values/styles.xml

@@ -207,6 +177,7 @@
        <item name="android:layout_gravity">left</item>
        <item name="android:gravity">left|center_vertical</item>
        <item name="android:typeface">normal</item>
+        <item name="android:foreground">?android:attr/selectableItemBackground</item>


Another day, another new trick I learned!

Yeah, I was really happy when I saw this in a gnarly SO post.

SUPERCILEX

@samtstern Make sure to read this post first for the others to make sense. Happy reading! 😄

SUPERCILEX · 2017-05-09T19:48:00Z

auth/src/main/res/values/colors.xml

-    <color name="authui_inputTextColorDark">#000000</color>
+    <color name="colorPrimary">#3F51B5</color>
+    <color name="colorPrimaryDark">#303F9F</color>
+    <color name="colorAccent">#FF4081</color>


@samtstern Wait a sec, I think you're confused by the purpose of this PR and the pure awesomeness going on behind the scenes! 😄 Those colors don't mean anything are the default Android Studio setup colors:

Hex value Color

<color name="colorPrimary">#3F51B5</color>

<color name="colorPrimaryDark">#303F9F</color>

<color name="colorAccent">#FF4081</color>

(I'm getting good at those tables 😆)

The colors you see from the screenshots in the original post are from the dev's app theme (in this case FirebaseUI): https://github.com/firebase/FirebaseUI-Android/blob/master/app/src/main/res/values/colors.xml#L2-L4

So the idea is that with no configuration at all, a dev can just add FirebaseUI and it will automatically style itslef with their app's theme because their colorPrimary, colorPrimaryDark, and colorAccent are overriding FirebaseUI's. Thus the colors in the FirebaseUI style are meaningless and are just there so that we can compile the auth module.

SUPERCILEX · 2017-05-09T19:50:27Z

auth/src/main/res/values/styles.xml

@@ -2,17 +2,10 @@
 <resources>

    <style name="FirebaseUI" parent="Theme.AppCompat.Light.DarkActionBar">
-        <item name="colorPrimary">@color/authui_colorPrimary</item>


Yup! To build on what I said above, this PR makes heavy usage of AppCompat which takes care of almost all of the styling for us using the dev's overriden colors.

SUPERCILEX · 2017-05-09T19:51:37Z

auth/src/main/res/values/styles.xml

        <item name="android:textSize">16sp</item>
-        <item name="android:lineSpacingExtra">8sp</item>


Not really, they're using @style/Widget.AppComat.TextInputLayout which takes care of the styling for us. (And of course AppCompat is following the material spec.)

SUPERCILEX · 2017-05-09T20:13:12Z

auth/src/main/res/values/styles.xml

@@ -207,6 +177,7 @@
        <item name="android:layout_gravity">left</item>
        <item name="android:gravity">left|center_vertical</item>
        <item name="android:typeface">normal</item>
+        <item name="android:foreground">?android:attr/selectableItemBackground</item>


Yeah, I was really happy when I saw this in a gnarly SO post.

firebase/FirebaseUI-Android#696 firebase/FirebaseUI-Android#694 Signed-off-by: Alex Saveau <[email protected]>

Massively simplify styling

fbb9196

SUPERCILEX commented May 9, 2017

View reviewed changes

samtstern reviewed May 9, 2017

View reviewed changes

SUPERCILEX commented May 9, 2017

View reviewed changes

SUPERCILEX added 3 commits May 9, 2017 13:38

Animate TextInputLayout hints

ee86569

Cleanup

b31f6e1

Remove demo app styling that's no longer necessary

18435dd

SUPERCILEX changed the title ~~[WIP] Massively simplify styling~~ Massively simplify styling May 9, 2017

samtstern added this to the 2.0.0 milestone May 9, 2017

samtstern merged commit f2a78b2 into firebase:version-2.0.0-dev May 9, 2017

SUPERCILEX deleted the simplify-styling branch May 9, 2017 22:23

SUPERCILEX added a commit to SUPERCILEX/Robot-Scouter that referenced this pull request May 10, 2017

Make login process way prettier and fix crash:

bc4009e

firebase/FirebaseUI-Android#696 firebase/FirebaseUI-Android#694 Signed-off-by: Alex Saveau <[email protected]>

SUPERCILEX added a commit to SUPERCILEX/Robot-Scouter that referenced this pull request May 23, 2017

Make login process way prettier and fix crash:

6976de0

firebase/FirebaseUI-Android#696 firebase/FirebaseUI-Android#694 Signed-off-by: Alex Saveau <[email protected]>

SUPERCILEX mentioned this pull request Jul 10, 2017

name of resources #794

Closed

SUPERCILEX mentioned this pull request Jul 22, 2017

Fix broken auto styling #815

Merged

WillieCubed mentioned this pull request Aug 9, 2017

Update README.md to include app_name and color requirements #862

Merged

Hex value	Color
`<color name="colorPrimary">#3F51B5</color>`
`<color name="colorPrimaryDark">#303F9F</color>`
`<color name="colorAccent">#FF4081</color>`

		<item name="android:textSize">16sp</item>
		<item name="android:lineSpacingExtra">8sp</item>

Massively simplify styling #696

Massively simplify styling #696

Uh oh!

Conversation

SUPERCILEX commented May 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samtstern left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SUPERCILEX May 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SUPERCILEX left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SUPERCILEX May 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SUPERCILEX May 9, 2017 •

edited

Loading

SUPERCILEX left a comment •

edited

Loading

SUPERCILEX May 9, 2017 •

edited

Loading