opacapp / multiline-collapsingtoolbar

A modified CollapsingToolbarLayout that can deal with multiline titles
Other
783 stars 114 forks source link

expandedTitleGravity / collapsedTitleGravity limited #6

Closed adamlachmann closed 8 years ago

adamlachmann commented 8 years ago

Hi,

The only attributes of expandedTitleGravity which I tested and that work are center_vertical, bottom and top. Using this library I was unable to center text horizontally when toolbar was expanded or collapsed.

I guess transition from centered multiline text to single left/right aligned line might be difficult to implement but it would certainly be welcomed feature.

johan12345 commented 8 years ago

Hi,

thanks for the bug report. If these settings work in the original CollapsingToolbarLayout in the Android Support Library, this is probably one of the features I broke with my various modifications to it for making the multiline text work. Other things I didn't test and may be broken could be things like RTL languages support etc.

Because we we don't use custom title gravity values the in our own app which we wrote this library for, fixing these issues is unfortunately not a high priority for us. But if you want to, you and other people encountering this bug are welcome to help fixing it and to open a Pull Request. If you need help, you can just ask here or via email at info@opacapp.de.

The code you want to look at is probably the CollapsingTextHelper class. Please note that all modifications I made to the original Support Library code are marked with comments to simplify updating the code with later changes in the Support Library. So these comments are probably also helpful to find the locations where you need to adjust things for making the gravity settings work. And if you do additonal adjustments at other locations, these should of course also be marked.

jsotuyod commented 8 years ago

The issue seems to be here

mTextLayout = new StaticLayout(mTextToDraw, mTextPaint, (int) availableWidth,
                    Layout.Alignment.ALIGN_NORMAL, 1, 0, false);

Alignment is always ALIGN_NORMAL, which defaults to START (you haven't broken RTL support! yay!)

Checking mExpandedTextGravity to set a proper flag should do the trick... will try to get it fixed and send a PR...

jsotuyod commented 8 years ago

After looking at it, it's not gonna be that simple.. you are overlaying the layout with a bare drawText and having those match up is not gonna be easy since they compute sizes based on differents strings (ellipsied and non-ellipsized), plus, the original helper manually centered strings for ease of transitioning...

The best course of action seems to be to drop StaticLayout altogether and draw multiline by hand...

johan12345 commented 8 years ago

Thank you for starting to look into fixing this!

Well, of course both the positions of both the expanded text (drawn using the StaticLayout) and the collapsed and cross-section texts (drawn using drawText) need to be adjusted to make the expanded text e.g. centered. I think drawing the multiline text by hand would be quite complex because one would need to rebuild all the logic that decides where the line breaks are put etc. I'd rather suggest to use methods like [getLineBounds](https://developer.android.com/reference/android/text/Layout.html#getLineBounds%28int, android.graphics.Rect%29) on the StaticLayout to get the first line's position and then use that to find out where to position the other pieces of text using drawText.

jsotuyod commented 8 years ago

@johan12345 that won't work... getLineBounds is designed to get the baseline of a line, left and right will be 0 and the StaticLayout's width respectively regardless of the text's length and alignment.

johan12345 commented 8 years ago

@jsotuyod Oh, I see. But getLineLeft should work for that, shouldn't it?

jsotuyod commented 8 years ago

@johan12345 that's actually much more promising! Will tinker a little with it....

jsotuyod commented 8 years ago

Got it working! I'm tidying it up a little and doing some more tests, but so far so good. Next update will be a PR.

raphaelm commented 8 years ago

Included in release 1.2.1