photo_view icon indicating copy to clipboard operation
photo_view copied to clipboard

[RFC] Callbacks as Notifications

Open renancaraujo opened this issue 3 years ago • 3 comments

Is your feature request related to a problem? Please describe. Yes, we have a lot of options to be passed to photo view, and since most of them are proxied form the gallery widget, things are getting hard to maintain. There is (or should be) callbacks for the following events:

  • Gesture events:
    • double tap
    • tap down
    • tap up
    • scale start
    • scale update
    • scale end
  • State events:
    • loading error
    • loading finish
    • state update

That is a lot!

The traditional way of specifying callbacks involves the package user passing callbacks as parameters and these parameters are filled down to the very widget where the supposed event happens. On top of that, all these callbacks are nullable options, so we end up polluting our code with callback-related stuff.

Describe the solution you'd like TL;DR: Don't pass callbacks directly to the PV widget, but use the Notification API to bubble up events and abstract everything in a specialized widget.

This would affect two places:

  • The places where the events happen would emit the notifications.
  • These notifications could be listened by a specialized widget that would be optionally added wrapping the main PV widget.

Example:

return PhotoViewCallbacks(
  onDoubleTap: () {}
  child: PhotoView(...)
);

Or as a method:

return PhotoView(...).withCallbacks(
 onDoubleTap: () {}
);

This would make things easier to maintain and less cumbersome with all the options possible to e passed to photo view.

renancaraujo avatar Mar 23 '21 20:03 renancaraujo

I didn't know about this Notifications API, but coming from the context of the browser and how DOM handles the event propagation, I think it's a very cool solution. The API looks similar to what you get by using Flutter's GestureDector directly, which I guess you could do:

GestureDector(
  yourListener: foo,
  child: PhotoView(),
)

What would be the differences of having PhotoView capture and then propagate these events rather than just let the closest GestureDector handle it?

luanpotter avatar Mar 29 '21 17:03 luanpotter

I didn't know about this Notifications API, but coming from the context of the browser and how DOM handles the event propagation, I think it's a very cool solution. The API looks similar to what you get by using Flutter's GestureDector directly, which I guess you could do:

GestureDector(
  yourListener: foo,
  child: PhotoView(),
)

What would be the differences of having PhotoView capture and then propagate these events rather than just let the closest GestureDector handle it?

For starters not all PhotoView callbacks are from gesture events, there is the state callbacks. Also we have to know if an particular event has been handled by PhotoView, we may have particular use cases that may make a wrapping gesture detector notto reflect on PhotoViews event, like:

  • When there is an opaque gesture detector in the child when using PhotoView.customChild
  • when wrapping photo view gallery, it will be impossible to know which particular photo view instance I supposed to handle that particular event.

renancaraujo avatar Mar 29 '21 22:03 renancaraujo

I see; in that case, I think it makes a lot of sense to use the events api to basically extend this native functionality by wrapping and adding new events.

luanpotter avatar Mar 30 '21 13:03 luanpotter