Feature/scrollbars #30
Conversation
added implementations for scrollXXX methods (wip)
removed unnecessary tags
removed todo
|
Man thanks! I will take a look as soon as I can, nice work |
|
Travis is missing the local.properties file which is ignored by git. |
|
Yeah you can ignore that, I will fix it later |
| protected int computeHorizontalScrollExtent() { | ||
| Rect localRect = new Rect(); | ||
| getLocalVisibleRect(localRect); | ||
| return localRect.width(); |
There was a problem hiding this comment.
Is there any reason why using this rather than getWidth() and getHeight? It should return the same thing almost always. Also in vertical extent & in ZoomLayout callbacks.
If there is a reason, could you cache this rect in a tempRect field? So we don't create a lot of them.
There was a problem hiding this comment.
No, I think it should also work with getWidth() and getHeight().
I don't have that much experience with custom views so the "localVisibleRect" method looked like a good fit, but I guess it can be slow.
| } | ||
|
|
||
| onDrawScrollBars(canvas); |
There was a problem hiding this comment.
You can remove this and use setWillNotDraw(false) in the constructor.
| @@ -89,12 +90,50 @@ public boolean onTouchEvent(MotionEvent ev) { | |||
| public void onUpdate(ZoomEngine helper, Matrix matrix) { | |||
| mMatrix.set(matrix); | |||
| setImageMatrix(mMatrix); | |||
|
|
|||
| if (!awakenScrollBars()) { | |||
| invalidate(); | |||
There was a problem hiding this comment.
Any reason for this call? We should not need to invalidate. setImageMatrix will do it.
Same for ZoomLayout.
There was a problem hiding this comment.
The documentation for awakenScrollBars() states:
When the animation is started, this method returns true,
and false otherwise. If the animation is started, this method
calls {@link #invalidate()}; in that case
the caller should not call {@link invalidate()}.
So I added this condition. I guess if setImageMatrix already does it this check is useless.
There was a problem hiding this comment.
Yeah I think it's better to simply call awakenScrollBars() and ignore the result.
Also, you need to merge the other PR changes to merge this one. Thanks!
# Conflicts: # app/src/main/res/layout/activity_main.xml
|
I removed the if condition, merged master and added xml attributes for demo ZoomImageView. |
|
Thanks @markusressel ! I will release a new version today with your changes. |
* added xml attributes for both horizontal and vertical scrollbars added implementations for scrollXXX methods (wip) * scrollbars do work for symmetric but don't work yet for asymmetric content * fixed scrollbars for asymmetric content * added scrollbar xml tag to imageview removed unnecessary tags * moved companion methods closer together removed todo * fixed scrollbars getting rendered behind content * review fixes * review fix * merge fix
There is some reordering going on in the xml file, hope you don't mind.
This is a PR for #29