Keeping Styles in Scope

Dave Smith
Dave Smith
Keeping Styles in Scope

Styles and Themes are a great thing. They allow us to abstract common view properties into a single location, making the look and feel of our application UI more consistent and easier to maintain. If you look at Google's UI guide for Styles and Themes, it mentions the motivation behind this as a separation between design logic and content.

However, as with any abstraction API, it can be easy to take things a bit too far. Android does not restrict which attributes one can abstract into a style. Any attribute you can put on a view element, you can place in a style. But just because you can put something in a style doesn't mean you should.

What I hope to communicate here is that specifically adding LayoutParams to your style definitions is not a great idea, and complicates your code instead of simplifying it.

An Example

Let's start off with an example to illustrate the point. Imagine you are asked to review code written by someone else, or perhaps take over where they left off. You run the application for the first time and you see the following form layout:

Example Layout

You then jump into the activity layout and find this:

<RelativeLayout
    xmlns:android="http://schemas.android.com/apk/res/android"
    android:layout_width="match_parent"
    android:layout_height="match_parent">
    <Button
        style="@style/FormButton.Cancel"
        android:text="@android:string/cancel"/>
    <Button
        style="@style/FormButton.Ok"
        android:text="@android:string/ok"/>
    <TextView
        style="@style/LabelText.Username"
        android:text="@string/label_username"/>
    <TextView
        style="@style/LabelText.Password"
        android:text="@string/label_password"/>
    <EditText
        style="@style/FormText.Username"/>
    <EditText
        style="@style/FormText.Password"/>
</RelativeLayout>

Can you tell by analyzing the layout file whether the rendered view is correct? Are the text labels aligned properly? Is the button width appropriate? In order to answer questions like these we will have to examine res/values/styles.xml since they aren't apparent here.

<resources>
    ...

    <style name="LabelText" parent="android:TextAppearance.Medium">
        <item name="android:layout_width">wrap_content</item>
        <item name="android:layout_height">@dimen/label_height</item>
        <item name="android:paddingRight">@dimen/label_padding</item>
        <item name="android:textStyle">bold</item>
        <item name="android:gravity">bottom</item>
    </style>
    <style name="LabelText.Username">
        <item name="android:id">@id/label_username</item>
        <item name="android:layout_above">@id/label_password</item>
    </style>
    <style name="LabelText.Password">
        <item name="android:id">@id/label_password</item>
        <item name="android:layout_above">@id/button_ok</item>
    </style>

    <style name="FormButton" parent="android:Widget.Holo.Light.Button">
        <item name="android:layout_width">wrap_content</item>
        <item name="android:layout_height">wrap_content</item>
        <item name="android:layout_marginLeft">@dimen/button_margin</item>
        <item name="android:layout_marginRight">@dimen/button_margin</item>
        <item name="android:layout_alignParentBottom">true</item>
        <item name="android:background">@drawable/button_background</item>
        <item name="android:minWidth">96dp</item>
    </style>
    <style name="FormButton.Ok">
        <item name="android:id">@id/button_ok</item>
        <item name="android:layout_toLeftOf">@id/button_cancel</item>

    </style>
    <style name="FormButton.Cancel">
        <item name="android:id">@id/button_cancel</item>
        <item name="android:layout_alignParentRight">true</item>
    </style>

    <style name="FormText" parent="android:Widget.Holo.Light.EditText">
        <item name="android:layout_width">match_parent</item>
        <item name="android:layout_height">wrap_content</item>
    </style>
    <style name="FormText.Username">
        <item name="android:layout_toRightOf">@id/label_username</item>
        <item name="android:layout_alignBottom">@id/label_username</item>
        <item name="android:inputType">textEmailAddress</item>
    </style>
    <style name="FormText.Password">
        <item name="android:layout_toRightOf">@id/label_password</item>
        <item name="android:layout_alignBottom">@id/label_password</item>
        <item name="android:inputType">textPassword</item>
    </style>
</resources>

The above is completely valid code; as I said, styles will accept any attribute. This includes LayoutParams and even ids.

Now, as you are working, the question in your mind becomes "Is this style used anywhere else? It must have been abstracted for a reason." If so, making a change to adjust the layout here will affect those other layouts. If that effect is undesired, you'll end up having to spin off another style to accommodate the change.

The Issue is Context

...no, not that context. LayoutParams on a view by themselves have no meaning without the parent container. Any attribute that is prefixed with android:layout_ is meant to describe the relationship between the view itself and its parent. Therefore, every layout attribute only makes sense in the context of those two elements, and it follows that those attributes ought to be present in the layout files that define that relationship.

Following the same principle, it is not necessarily safe to abstract attributes from a view that are specific to a particular layout manager. This places an implicit constraint that the style that it will only be applied in certain containers. For example, placing android:layout_alignParentRight or android:layout_gravity in a style is dangerous because those attributes are not globally supported by all container views. The former is only allowed in RelativeLayout, which doesn't support the latter attribute at all.

Note: Sometimes I'll get pushback on this that only one container type is ever expected in the application. If that is the case, why go through the trouble of abstracting into a style at all for use in one place? Or if in multiple places, why not use a layout include or a custom view as a cleaner approach to code reuse?

In my opinion, this takes typing abstraction to the level where it actually negatively affects the maintainability of the code. It forces you to inspect the applied styles in concert with the layout to diagnose any possible issues related to the placement and sizing of views. The scope of a layout attribute should be just to that layout, not a style that will likely be applied across multiple layouts.

What About Just Height & Width?

I often get pushback on this from folks indicating that they only abstract android:layout_width and android:layout_height in styles since they are global to all layout managers. If any additional LayoutParams are necessary, those can be placed directly in the layouts themselves. This solves the problem of accidentally using attributes from the wrong layout manager, right?

Yes, it does...but it does nothing for the readability. In fact, I believe it makes it slightly worse. Let's rewrite the above example using this as a guiding principle.

<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
    android:layout_width="match_parent"
    android:layout_height="match_parent">
    <Button
        android:id="@+id/button_cancel"
        style="@style/FormButton"
        android:layout_marginLeft="@dimen/button_margin"
        android:layout_marginRight="@dimen/button_margin"
        android:layout_alignParentRight="true"
        android:layout_alignParentBottom="true"
        android:text="@android:string/cancel"/>
    <Button
        android:id="@+id/button_ok"
        style="@style/FormButton"
        android:layout_marginLeft="@dimen/button_margin"
        android:layout_marginRight="@dimen/button_margin"
        android:layout_toLeftOf="@id/button_cancel"
        android:layout_alignParentBottom="true"
        android:text="@android:string/ok"/>
    <TextView
        android:id="@+id/label_username"
        style="@style/LabelText"
        android:layout_above="@id/label_password"
        android:text="@string/label_username"/>
    <TextView
        android:id="@+id/label_password"
        style="@style/LabelText"
        android:layout_above="@id/button_ok"
        android:text="@string/label_password"/>
    <EditText
        android:id="@+id/text_username"
        style="@style/FormText.Username"
        android:layout_toRightOf="@id/label_username"
        android:layout_alignBottom="@id/label_username"/>
    <EditText
        android:id="@+id/text_password"
        style="@style/FormText.Password"
        android:layout_toRightOf="@id/label_password"
        android:layout_alignBottom="@id/label_password"/>
</RelativeLayout>

The problem you might already see is that, at first glance, the LayoutParams in this file look complete until you stare at it for a moment. So as the outside observer, you might expect everything you need to be here in front of you. Only to be confused shortly thereafter when you realize android:layout_width and android:layout_height are missing. So we still have to open styles.xml to get the full picture of what is going on with the full layout:

<resources>
    ...

    <style name="LabelText" parent="android:TextAppearance.Medium">
        <item name="android:layout_width">wrap_content</item>
        <item name="android:layout_height">@dimen/label_height</item>
        <item name="android:paddingRight">@dimen/label_padding</item>
        <item name="android:textStyle">bold</item>
        <item name="android:gravity">bottom</item>
    </style>

    <style name="FormButton" parent="android:Widget.Holo.Light.Button">
        <item name="android:layout_width">wrap_content</item>
        <item name="android:layout_height">wrap_content</item>
        <item name="android:background">@drawable/button_background</item>
        <item name="android:minWidth">96dp</item>
    </style>

    <style name="FormText" parent="android:Widget.Holo.Light.EditText">
        <item name="android:layout_width">match_parent</item>
        <item name="android:layout_height">wrap_content</item>
    </style>
    <style name="FormText.Username">
        <item name="android:inputType">textEmailAddress</item>
    </style>
    <style name="FormText.Password">
        <item name="android:inputType">textPassword</item>
    </style>
</resources>

Bottom line: You shouldn't have to open styles.xml to diagnose an issue with the layout XML. You also shouldn't have to worry about whether a change made in styles.xml to fix a specific layout problem is going to propagate to other layouts, possibly breaking something elsewhere, since "there must have been a reason to abstract it to a common location.

Note: This still forces an implicit constraint that you always want the size of the widget to be consistent along with the styling. Layouts should be free to define size and placement, styles should be free to govern the look and feel regardless of what size the layout requests.

Common Arguments

To conclude, listed here are other miscellaneous thoughts I've heard on the subject that I felt were worth addressing.

Hey dummy, the UI guide you linked does this exact same thing. It's obviously a sanctioned method.
...don't blindly trust DEBB. Just because you saw it in an example, even one from Google, doesn't make it a good idea. If you look at Google's code (i.e. the core styles.xml file in AOSP) you will notice that this is a practice done rarely, and they are scoped to very specific situations (such as the widgets that make up the action bar).

I am the only developer on this project, and I'll remember how I've structured my own code. Spending time making my code more readable to others is a waste.
...today, perhaps; but tomorrow? Who knows? Besides, even if you are a 100% solo operation, realize that we are all morons. Your memory isn't as good as you think it is; let's try to do things that won't piss off our future selves.

What about margins? Those are tied to the spacing we always want applied to the widget.
...that's a valid argument, but not all layout containers support margins and you may find yourself in a situation where margins aren't respected. This is why the framework bakes most widget spacings into the asset backgrounds (Button, EditText, etc. all do this).

That's so much extra typing!
...code completion was invented to solve this problem, not style abstraction.