JakeWharton / ViewPagerIndicator

Paging indicator widgets compatible with the ViewPager from the Android Support Library and ActionBarSherlock.
http://viewpagerindicator.com
10.14k stars 4.01k forks source link

CirclePageIndicator not properly centered #281

Open kdbruin opened 10 years ago

kdbruin commented 10 years ago

I've a layout where a clear line marks the vertical center of the screen:

https://www.dropbox.com/s/pa7mfx73liujz6b/device-2014-03-11-154308.png

As you can see the indicators do not center properly with either the "centered" or "layout_gravity" attribute. I've the following XML:

<FrameLayout
    android:layout_width="match_parent"
    android:layout_height="0dp"
    android:layout_weight="1" >

    <android.support.v4.view.ViewPager
        android:id="@+id/pager"
        android:layout_width="match_parent"
        android:layout_height="match_parent"
        android:layout_gravity="center" />

    <com.viewpagerindicator.CirclePageIndicator
        android:id="@+id/indicator"
        android:layout_gravity="bottom|center"   <!-- doesn't work -->
        android:layout_width="fill_parent"
        android:layout_height="wrap_content"
        android:background="@android:color/transparent"
        android:padding="6dp"
        app:centered="true"    <!-- doesn't work either -->
        app:fillColor="#FFFFFFFF"
        app:pageColor="#88FFFFFF"
        app:radius="4dp"
        app:strokeColor="#FF1047A9"
        app:strokeWidth="1dp" />
</FrameLayout>

Changing the layout_width for the indicator to "wrap_content" doesn't change anything.

This is with version 2.4.1 of the library.

console23 commented 10 years ago

This is both a non-bug and a bug. It is a non-bug because the custom view class uses the onDraw() method to draw circles directly onto the Canvas, and so it does not utilize the layout_ properties. Technically, it probably should heed those. I personally have seen this because I tried layout_gravity="center".

In lieu of this, I suggest you add padding to the sides so the dots don't get clipped. Example from my layout file: <LinearLayout android:layout_width="match_parent" android:layout_height="24dp" android:orientation="horizontal" >

    <TextView
        android:id="@+id/news_articleHeader"
        android:layout_width="0dp"
        android:layout_height="match_parent"
        android:layout_weight="1"
        android:text="@string/articles"
        android:gravity="left|center_vertical"
        android:paddingLeft="12dp"
        android:textColor="#ffffff"
        android:background="#1010ff" />

    <com.viewpagerindicator.CirclePageIndicator
        android:id="@+id/news_scrollView_pageIndicator"
        android:layout_width="wrap_content"
        android:layout_height="match_parent"
        android:background="#1010ff"
        android:paddingLeft="8dp"
        android:paddingRight="8dp" />

</LinearLayout>

HOWEVER, I have found another bug that is related to this ignorance of the layout properties. The class supports a method for calling setCentered(boolean) to specifically allow centering. It is not utilized in the CirclePageIndicator code properly, causing the dots to appear at the top edge of your layout (i.e. LinearLayout, or what have you). I have made the following code block change in my local repository, for CirclePageIndicator::onDraw():

protected void onDraw(Canvas canvas) {
...
        //Draw stroked circles
        for (int iLoop = 0; iLoop < count; iLoop++) {
            float drawLong = longOffset + (iLoop * threeRadius);
            if (mOrientation == HORIZONTAL) {
                dX = drawLong;
                //dY = shortOffset; /*commented out by console23*/
                dY = (mCentered == true ? (this.getHeight() / 2) : shortOffset); /*added by console23*/
            } else {
                //dX = shortOffset; /*commented out by console23*/
                dX = (mCentered == true ? (this.getWidth() / 2) : shortOffset); /*added by console23*/
                dY = drawLong;
            }
...
        //Draw the filled circle according to the current scroll
        float cx = (mSnap ? mSnapPage : mCurrentPage) * threeRadius;
        if (!mSnap) {
            cx += mPageOffset * threeRadius;
        }
        if (mOrientation == HORIZONTAL) {
            dX = longOffset + cx;
            //dY = shortOffset; /*commented out by console23*/
            dY = (mCentered == true ? (this.getHeight() / 2) : shortOffset); /*added by console23*/ 
        } else {
            //dX = shortOffset; /*commented out by console23*/
            dX = (mCentered == true ? (this.getWidth() / 2) : shortOffset); /*added by console23*/
            dY = longOffset + cx;
        }
        canvas.drawCircle(dX, dY, mRadius, mPaintFill);
    }

I hope this helps someone else. If so, please kindly register/sign-in and comment, and add any other information that may benefit others here.

kdbruin commented 10 years ago

@console23 I've tried your changes to the library code but still the indicators aren't properly centered on the screen. I will look further into this when I have some spare time on my hands...

donyu commented 10 years ago

@kdbruin I'm having the same problem as you. Did you get a chance to look further into this? I'll try to take a stab at getting this fixed too

lastquestion commented 10 years ago

This is an easy fix. The code does not center correctly because it doesn't take into account how the circles are actually drawn. See https://github.com/lastquestion/Android-ViewPagerIndicator/commit/4447b1b472b68effd7b60726f567d75c42b32e07

desseim commented 9 years ago

@lastquestion was on the right track but I think but his version failed to take padding into account. The special case for 1 circle seemed to be unnecessary as well.

I've modified the onMeasure() to center properly taking padding into account, for those interested it's here. I've only checked it to behave correctly with 1, 2 or 3 pages, with and without paddings, and with a wrap_content as layout_width, so it may still be buggy in some other configuration.

Also note, if you lay it out as wrap_content in width, that the widget has to be re-laid out upon data change, otherwise it won't resize when the view pager or its content data changes to a different page count.

addwittynamehere commented 9 years ago

I'm struggling with this too! This is probably a stupid question (and will probably be ignored considering this post is dead) but I don't know how to modify the actual CirclePageIndicator, any changes I make don't actually go through. If it means anything, the icons for all the ViewPagerIndicator classes are grayed out and have a red J.

msdx commented 9 years ago

I think @JakeWharton may be not good at math(^_^). I also found this issue when I used it, and fixed it at https://github.com/msdx/ViewPagerIndicator/commit/8cffd7d5d19542413b2900baf31ff7375941c74b, which is based on dev branch.

ekchang commented 8 years ago

My variant of the fix: if (mCentered) { longOffset = (longSize / 2.0f) - ((count * 2 * mRadius) + (count - 1) * mSpacing) / 2.0f + mRadius; } Where I have mSpacing for custom spacing between circles. Full code can be seen at ekchang/ViewPagerIndicator/CirclePageIndicator.java

msdx commented 8 years ago

@ekchang Our code on different branches. In dev branch, the author added a field mGap for custom spacing between circles.

salhugues commented 7 years ago

All that you have to do is to replace this line: if (mCentered) { longOffset += ((longSize - longPaddingBefore - longPaddingAfter) / 2.0f) - ((count* threeRadius) / 2.0f); }

By: if (mCentered) { longOffset = 0; longOffset += ((longSize - longPaddingBefore - longPaddingAfter) / 2.0f) - (((count - 1) * threeRadius) / 2.0f); }

And in xml the width of your Page indicator need to match_parent.

It worked for me.