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

Controller API allow internal malfunction #127

Closed
renancaraujo opened this issue Apr 19, 2019 · 1 comment
Closed

Controller API allow internal malfunction #127

renancaraujo opened this issue Apr 19, 2019 · 1 comment
Labels
bug Something isn't working

Comments

@renancaraujo
Copy link
Member

renancaraujo commented Apr 19, 2019

Back on #89 we introduced controller, which allows authors to listen for updates in PhotoViews internal state and update it externally.
The value of the state is represented by PhotoViewControllerValue. The problem is that two fields of this class are semantically very tied to each other: scale and scaleState.

Scale state represents the step of the "scaleStateCycle" in which the widget is currently at. Changes in this cycle are triggered by the "doubleTap" user gesture. This value is basically a representation of the scale and it is very dependent on the size of the widget and the size of the image.

A change in scaleState should change the scale (internally it is made with an animation) calculated based on the size of the widget and the size of the content.

Any change in scale should set a scaleState to zoomingIn or zoomingOut based on its position in relation to PhotoView.initialScale.

Any update on only one of those values will make a PhotoView widget buggy and unpredictable.

This situation led to the following line in the controller.scale documentation:

Important: Avoid setting this field without setting scaleState to PhotoViewScaleState.zooming.

This is delegating to the author a logic that is supposed to be internal.

Considering

  • scaleState is a representational value and should not be allowed to be set without changing scale afterward.
  • The effect you get on scale after setting scaleState should be handled internally (PhotoViewImageWrapper animation)
  • Changes in scale should affect scaleState.
  • Since in most cases, both the size of the widget and the content are dynamically set and sometimes can only be retrieved after the first layout, there is no way to
    -scaleState does not belong to PhotoViewControllerValue because it is dependent on the size of the content and widget.

A new Controller API has to be made.

@renancaraujo renancaraujo added the bug Something isn't working label Apr 19, 2019
@renancaraujo
Copy link
Member Author

Solved on #129

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant