Skip to content
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

Add parsing of independent padding attributes #69

Merged
merged 1 commit into from
Feb 21, 2015

Conversation

ataulm
Copy link

@ataulm ataulm commented Feb 20, 2015

In our app, we require the ability to set a different value for the right padding (we want 0). Although the material guidelines specify that it's possible to add an offset to indicate that the tab strip is scrollable, there are no written guidelines written that mandate the right padding should equal the left padding.

This change allows users to specify the left and right paddings independently.


<com.astuetz.PagerSlidingTabStrip
    android:id="@+id/tabs"
    android:layout_width="match_parent"
    android:layout_height="?attr/actionBarSize"
    android:background="?attr/colorPrimary"
    app:pstsPaddingMiddle="true" />

after_as_per_sample


<com.astuetz.PagerSlidingTabStrip
    android:id="@+id/tabs"
    android:layout_width="match_parent"
    android:layout_height="?attr/actionBarSize"
    android:background="?attr/colorPrimary" />

after_no_padding_specified


<com.astuetz.PagerSlidingTabStrip
    android:id="@+id/tabs"
    android:layout_width="match_parent"
    android:layout_height="?attr/actionBarSize"
    android:background="?attr/colorPrimary"
    android:paddingLeft="48dp"
    android:paddingRight="48dp" />

after_padding_left_and_right


<com.astuetz.PagerSlidingTabStrip
    android:id="@+id/tabs"
    android:layout_width="match_parent"
    android:layout_height="?attr/actionBarSize"
    android:background="?attr/colorPrimary"
    android:paddingLeft="48dp" />

after_padding_left_only

- Allows paddingLeft and paddingRight to be set separately
- For existing apps that _only_ specify left or right, they must add
the other property for no change to be effected
- For existing apps that specify the same padding for left and
right, they needn't do anything to maintain behaviour
- For existing apps that specify different values for left and right
padding, they must increase the smaller padding to match the
higher one to maintain behaviour
- For existing apps that use `app:pstsPaddingMiddle="true"`
no change is required to maintain behaviour (this seems to
override the padding values anyway)
int paddingRight = a.getDimensionPixelSize(PADDING_RIGHT_INDEX, padding);
int padding = a.getDimensionPixelSize(PADDING_INDEX, 0);
paddingLeft = padding > 0 ? padding : a.getDimensionPixelSize(PADDING_LEFT_INDEX, 0);
paddingRight = padding > 0 ? padding : a.getDimensionPixelSize(PADDING_RIGHT_INDEX, 0);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as per typical Android views, if android:padding is given, it will override android:padding[Side]

@ataulm
Copy link
Author

ataulm commented Feb 20, 2015

❗ depending on usage, this is a behaviour changing commit:

No change
For existing apps that specify the same padding for left and right, they needn't do anything to maintain behaviour
For existing apps that use app:pstsPaddingMiddle="true" no change is required to maintain behaviour (this seems to override the padding values anyway)

Change
For existing apps that only specify left or right, they must add the other property for no change to be effected
For existing apps that specify different values for left and right padding, they must increase the smaller padding to match the higher one to maintain behaviour

@ouchadam
Copy link

👍 for those awesome gif examples

@jpardogo
Copy link
Owner

Cheers for the PR, I will try to find some time this weekend to review PR's and bugs.

Regarding the this PR. I developed it like that in the first place, to be honest I don't know why I changed it. It is better to have both options. 👍 to the awesome gifs as well

@ataulm
Copy link
Author

ataulm commented Feb 20, 2015

cool, let me know if you want changes / I don't mind if you checkout my branch and commit on top of it to get it to a state where you're happy to merge in.

@jpardogo
Copy link
Owner

I will just update the README to let people know what they need to do with their padding attributes now as you well explained in the commit.

selfie-0

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants