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

Fix#415 #417

Merged
merged 3 commits into from
May 25, 2017
Merged

Fix#415 #417

merged 3 commits into from
May 25, 2017

Conversation

p-lr
Copy link
Collaborator

@p-lr p-lr commented May 24, 2017

Hey Mike,

I don't know if you're aware of #415 : the OP describes the problem pretty well. To summarize, when the MinimumScaleMode#FIT was introduced, the logic to calculate the constrained X/Y scroll position needed to be adapted. Well, not exactly. In fact, in most cases there is no issue. But when a developer extends the TileView and overrides (as he's allowed to) the getScrollMinX and getScrollLimitX to return something different than 0, then the whole TileView is shifted at minimum scale.

Investigating on this, i found that it was not the only issue. Scrolling slowly at the boundaries of the TileView would make the scroll position change from eg -100 to 0 without any transition (which is a weird behavior). This happens when the scale is equal to mMinimumScaleY or mMinimumScaleX, depending on the shape of the tileset.

So i worked on the logic of getConstrainedScrollX and getConstrainedScrollY to work also when non zero return values are defined for getScrollMinX, getScrollLimitX, getScrollMinY, getScrollLimitY.
I came up with this implementation, which i tested and so did the author of #415.

@moagrius
Copy link
Owner

+1 can you bump version, update readme and merge? i'll pull and upload to jcenter asap

@moagrius
Copy link
Owner

@peterLaurence i posted this in another PR a while back, ICYMI:

@peterLaurence are you following this? can you confirm all's well with the repo at this point? also, i'll be ready to get started on v3 in about 3 weeks, when summer term starts. first thing i'd like to do is the base component - a two-dimensional scroller. ZoomPanLayout will extend that as it's own component, for things like zoomable images or grids. My hope is that each component can exist separately on top of each other, until we finally get to pluggable things like paths, markers, hotspots, etc. anything in particular you'd like to achieve? we should probably move this out of issues, or at least out of this issue - how do you want to coordinate? wiki, or feature issues, or something else?

@p-lr
Copy link
Collaborator Author

p-lr commented May 25, 2017

I don't know how i missed your message above.. sorry for that.
Ok for your strategy to decouple first. To coordinate, i find the "Projects" tab excellent. You define a new project, say "v3", then you can add columns e.g ("To Do", "In Progress", "Done"). Each columns can receive cards describing the task. Each card can be converted into an issue, and moved to other columns. This gives an overview of the project, while still using issues for details.

As for things i'd like to achieve : have a mode that loads tiles so that the user don't even see the loading. But something efficient (like don't just extend the viewport). I don't know if its feasible. We will see :)

@p-lr p-lr merged commit 0aaaeb5 into moagrius:master May 25, 2017
@p-lr p-lr deleted the fix#415 branch May 25, 2017 09:40
@moagrius
Copy link
Owner

Sounds great. I created a project, will start to fill it in tonight. I'll also publish the latest to bintray tonight. Also, I like your idea - we might not be able to get it perfect, but I get we can come close. I'll ping you later. Thanks @peterLaurence

@p-lr
Copy link
Collaborator Author

p-lr commented May 25, 2017

Ok great! I will look into that tomorrow when I get time.
The timing is perfect for me, I have finished some personal dev work so I have some spare time.

@moagrius
Copy link
Owner

sounds great

also, 2.2.7 is on jcenter

thanks for the fix

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.

2 participants