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

center zoom around focused point... #277

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

center zoom around focused point... #277

wants to merge 3 commits into from

Conversation

marunjar
Copy link
Contributor

@marunjar marunjar commented Feb 7, 2016

... instead of always keeping top hour while zooming

@entropitor
Copy link
Contributor

Personally, I find that the original zooming gives better results in most of the cases.

Most of the time, the point I'm looking at is more to the top of the screen (somewhere between 1/4 and 1/2 of the screen). So when I zoom out with your changes, The event (and especially it's title) goes off the screen, which seems undesirable to me.

Maybe have a focused point that's a bit higher (and maybe customizable?)

@marunjar
Copy link
Contributor Author

marunjar commented Feb 9, 2016

Customizeable would be the 'easiest' solution.
Is it enough to set it as a fraction app:focusPoint="30%" or do you also want a dimension app:focusPoint="150dp"? The difference would be if you change device orientation.

Current default is then would be 0% or 0dp

@entropitor
Copy link
Contributor

I feel like percentage is probably a better solution.

@marunjar
Copy link
Contributor Author

done, i used 2 attributes for this

  • fixedFocusPointEnabled The focused point (time) while zooming the week view. You can declare it as a fraction app:focusPoint="30%" and if is not declared the top of the view is used.
  • fixedFocusPointFraction If you disable the fixed focus point the center of your zoom gesture is used.

@entropitor
Copy link
Contributor

  • You seem to have reverted AllDayEventHeight from dimension to int
  • The attributes are not totally clear to me yet. Can you make it a bit more clear?
  • Why are you adding the WeekViewUtil "prefixes" again?

marunjar added 2 commits February 17, 2016 19:58
allow setting a customizeable focus point (height) to center zoom:
- fixedFocusPointFraction: a fraction where to set the specific point in view height
- fixedFocusPointEnabled: if disabled the center of the zoom gesture is used
@marunjar
Copy link
Contributor Author

What do you mean by 'not totally clear'? Should i rename them or do you need more description and comments?

With fixedFocusPoint i wanted to point out the focused point around we zoom.
You can enable it to have a fixed point, if you disable it zoom is focused around your zoom gesture.

Edit: while reading this i think the name is not the best. how about fixedZoomOriginEnabled and fixedZoomOriginPos? Any other suggestions?

@entropitor
Copy link
Contributor

The description should be more clear. I'm for example confused by fixedFocusPointFraction and focusPoint. And why there is a need for fixedFocusPointEnabled. And why the description for focusPoint seems to be added next to fixedFocusPointEnabled.

I'm guessing you mean:

  • focusPoint The focused point (percentage of the vertical screen area) which will be used while zooming the week view. You can declare it as a fraction app:focusPoint="30%". If is not declared the top of the view is used.
  • fixedFocusPointEnabled I'm not sure what this is for? Is this the same as not filling in the app:focusPoint? If so, I'd leave it out.
  • fixedFocusPointFraction: not sure what this is for.

@marunjar
Copy link
Contributor Author

you're guessing right.
i changed the property names and description, hope it's a little clearer now.

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